[IV 22-23] : Objetivo 6 #72
Conversation
danielsp13
added this to the [M2] Implementación de algoritmo de corrección automática de exámenes (Lógica de negocio) milestone
faltaba nltk
no se había terminado de escribir correctamente
únicamente se hace para decidir si cambiar las tareas de cada Sistema CI
en contenedor de versión 3.12, parece que no hay gcc, investigar tras resultado de esto.
lo mismo que se ha hecho para cirrus
no era correcto restringir sólo por ficheros python, en caso de que se incluyan otros ficheros diferentes no se lanzarían
only-if sigue ejecutando cuando no debe hacerlo
elimina comentarios inline de tests innecesarios
|
Buenos días @JJ / revisor que vaya (o quiera) mirar este PR. Está listo para una revisión. Muchas gracias. |
There was a problem hiding this comment.
¡Buenas de nuevo Daniel!
Me alegro que consiguieses alcanzar el objetivo 5 bastante rápido pese a esos problemillas que surgieron.
Le he echado un vistazo a tu PR y la verdad es que lo veo todo bien, solo te he comentado una cosilla que entiendo que ha sido una confusión puntual. Mucha suerte!
docs/sistemas_ci.md
Outdated
|
|
||
| Se realizarán las pruebas en las siguientes versiones de Python, donde se han comparado todas las que hay en [Status of Python Versions](https://devguide.python.org/versions/), y se ha decidido tomar las que sean más interesantes para el testeo de la aplicación: | ||
|
|
||
| * **Python 3.11.1:** Esta versión es la que está en el *Contenedor Docker*, por lo que se hará esta comprobación en Cirrus. (Esta versión es la última lanzada que es estable). |
There was a problem hiding this comment.
Creo que esto lo has puesto al revés si no estoy yo equivocado. Ya que según he visto estás testeando desde docker usando GitHub Actions, por lo que presupongo que esta versión es la que tienes en docker ya que usas como imagen base la última versión estable.
There was a problem hiding this comment.
❗ totalmente cierto, al final lo acabé haciendo al revés y se me pasó hacer este cambio. gracias!!
| @@ -3,17 +3,13 @@ | |||
| from supercatch.corrector import * | |||
| from supercatch.examen import Modelo, Examen | |||
|
|
|||
| #===================================================================== | |||
There was a problem hiding this comment.
Y bueno sobre esto es lo mismo que en el otro objetivo, no debería aparecer ya que no tiene relación, pero al ser simples comentarios dudo que tenga importancia. 👍
There was a problem hiding this comment.
Esto lo único por lo que lo hice fue para pasar los tests a la hora de entregarlo, porque necesitaba que un último commit comprobase tareas (ya que las comprobaciones se lanzan únicamente cuando los cambios incluyen ficheros de código y no he hecho nada de esto en esta rama). Tuve en cuenta lo que me dijiste en el objetivo pero no tenía claro si cualquier commit servía así que por eso decidí hacerlo sobre esta rama por si la liaba.
|
@JJ, espero tu aprobación. 😄 |
.cirrus.yml
Outdated
| image: python:${PYTHON_VERSION}-slim | ||
| install_script: | | ||
| apt-get update | ||
| apt-get install gcc libffi-dev -y |
There was a problem hiding this comment.
a la hora de probarlo con github actions, no he tenido este problema porque al parecer la imagen que usa sí que o tiene un compilador, o lo resuelve de otra forma, mucho más lenta he de decir, me ha llegado a tardar 3 minutos, y realmente creo que sí me interesa probar esta versión que es la que está en desarrollo
There was a problem hiding this comment.
y como está en desarrollo, relacionado con ese issue, estaban trabajando en una opción que permita la instalación como se ha venido haciendo en todas las versiones de python (pero está en discusión todavía)
There was a problem hiding this comment.
el problema viene de la instalación de poetry (se haga vía pip o curl), a partir de una dependencia llamada cffi, que es la que lanza estos problemas. lo único que se me ocurre es explorar si puedo instalar esta dependencia directamente.
There was a problem hiding this comment.
o no tener en cuenta esta versión y usar una por debajo de la 3.11, pero entre elegir una versión aleatoria sin ningún interés y la que está desarrollo, creo que está claro.
There was a problem hiding this comment.
el problema viene de la instalación de poetry (se haga vía pip o curl), a partir de una dependencia llamada cffi, que es la que lanza estos problemas. lo único que se me ocurre es explorar si puedo instalar esta dependencia directamente.
De hecho voy a investigar esto que realmente creo que es lo mejor..., lo abordaré en un issue relacionado con el que le he indicado.
There was a problem hiding this comment.
el problema viene de la instalación de poetry (se haga vía pip o curl), a partir de una dependencia llamada
cffi, que es la que lanza estos problemas. lo único que se me ocurre es explorar si puedo instalar esta dependencia directamente.
OK, entonces es por poetry, no tanto por la versión. Tiene sentido, aunque es extraño que cirrus no se prepare para eso.
There was a problem hiding this comment.
A menos que cffi se construya desde el fuente, tampoco veo la necesidad de que instales eso. Una opción es siempre ver la biblioteca que falla y ver si está empaquetada para Ubuntu o Debian. Muchas veces te evitas problemas de esa forma. Incluso, en este caso, instalar la versión de poetry que está empaquetada, siempre que use el Python del sistema (que supongo que lo hará)
There was a problem hiding this comment.
el problema viene de la instalación de poetry (se haga vía pip o curl), a partir de una dependencia llamada
cffi, que es la que lanza estos problemas. lo único que se me ocurre es explorar si puedo instalar esta dependencia directamente.OK, entonces es por
poetry, no tanto por la versión. Tiene sentido, aunque es extraño que cirrus no se prepare para eso.
A mi también y sobre todo cuando al principio tenía pensado hacer estas comprobaciones con GitHub Actions. Acabé haciéndolo, y no me produjo ahí ningún tipo de fallo, salvo que la instalación de poetry tarda muchísimo en comparación con la versión 3.7, pero al fin y al cabo termina instalándolo todo sin problemas.
.cirrus.yml
Outdated
| @@ -0,0 +1,16 @@ | |||
| task: | |||
| name: Verify Tests | |||
| skip: "!changesInclude('**.py')" | |||
There was a problem hiding this comment.
Puede haber cambios en las dependencias que hay que testear también.
There was a problem hiding this comment.
no se me había ocurrido la verdad, lo cambiaré.
|
@JJ, he corregido el cambio que me ha indicado. Con respecto a lo que hemos hablado referente a #76 (abordado en #78) he comprobado que la mejor forma de solucionarlo es como lo tengo actualmente, al menos hasta que los desarrolladores de He probado eso, y también utilizar otras imágenes base, donde se resuelve el problema en aquellas que disponen de gcc (las versiones más reducidas como slim, no dispone de esto). |
En general, debes tratar de proporcionar una solución mínima y viable a los problemas. ¿Has mirado si, como te digo, está empaquetado cffi para Debian? ¿Has visto esto? ¿O |
es necesario especificar el path
|
Sólo se ejecutan los tests en Cirrus, no se ejecutan los tests en GitHub actions. En "checks passed" sólo muestra dos de ellos. |
There was a problem hiding this comment.
Aparte de lo encontrado, sigo viendo un problema que tengas que instalar gcc cada vez. Eso hace que un test tarde casi un minuto, de los cuales sólo un segundo es para el test en sí. Hay varias cuestiones
- Nadie te obliga a que uses Cirrus. Si uno de tus requisitos es usar la versión rc de Python, deberías usar el runner en el que se pueda instalar sin necesidad de instalaciones adicionales.
- Incluso aunque necesites hacerlo, estás ejecutando la matriz de versiones en Cirrus, ejecutando esa instalación en versiones (la 3.7) donde no necesitas
- Incluso aunque necesites testear esa versión, tampoco te obliga nadie a que la ejecutes de esta forma. Lo más razonable para estas versiones es, precisamente, usar contenedores de test, que tendrían todo lo necesario instalado en la imagen y no tardarían un minuto en ejecutarse
Recuerda que no es la elección de herramienta la que tiene que condicionar tu configuración. Es tu configuración la que tiene que condicionar la elección de herramienta, de forma que se elija la más adecuada para las diferentes pruebas que se quieren hacer.
Si no te queda otro remedio que instalar una serie de herramientas (que no creo que sea el caso) conviene que configures una caché. Si lo que tarda mucho en la instalación es la parte de Python, seguro que es sencillo configurarla.
| on: | ||
| push: | ||
| paths: | ||
| - '**.py' |
There was a problem hiding this comment.
Estos paths tendrías que modificarlos de la misma forma que el otro.
En general no, pero en este caso tengo que ir viendo check por check si se han ejecutado los dos o no. Normalmente cuando se está probando algo se incluyen los ficheros que se estén probando, en este caso los ficheros propios de los sistemas de CI. Luego se puede quitar después de fusionarlo. |
Estoy de acuerdo, optaré por el segundo punto de lo que me comenta, que es lo que también he probado, porque el problema no es de cirrus es de utilizar una imagen que tenga ya todo lo que necesite como dice. |
Vale, los siguientes cambios inlcuirán eso también para que lo pueda ver. Le volveré a mencionar cuando tenga todo listo. |
también se añade al path de Github Actions los ficheros de configuración de CI, sólo para testear.
es en lo que más tarda en instalar de las dependencias de poetry
|
@JJ resuelto. he combinado el uso de caché con una imagen que contiene directamente lo que se necesita. |
mundo?
integración continua más conveniente y se han calificado los sistemas que
cumplan esos requisitos según esos criterios?