danielsp13 / CorrectIt

CorrectIt: un corrector de exámenes escritos en lenguaje natural.
GNU General Public License v3.0
0 stars 0 forks source link

[IV 22-23] : Objetivo 6 #72

Closed danielsp13 closed 1 year ago

danielsp13 commented 1 year ago
danielsp13 commented 1 year ago

https://github.com/danielsp13/SuperCatch/pull/72/commits/7c407db7c093e1dd9e5721a2eb06bde80e868df3 y https://github.com/danielsp13/SuperCatch/pull/72/commits/faff3996643b5daa05e8f90daf34e4e7f2f4337b realmente refieren a #75

danielsp13 commented 1 year ago

Buenos días @JJ / revisor que vaya (o quiera) mirar este PR. Está listo para una revisión. Muchas gracias.

danielsp13 commented 1 year ago

@JJ, espero tu aprobación. :smile:

danielsp13 commented 1 year ago

@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 cffi lancen un paquete que no requiera de compilación previa (no hay ahora mismo para esta versión de python) o se resuelva la discusión actual sobre incluir una opción en la forma de instalar los paquetes de python como en todas las versiones existentes hasta la 3.11.

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).

JJ commented 1 year ago

@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 cffi lancen un paquete que no requiera de compilación previa (no hay ahora mismo para esta versión de python) o se resuelva la discusión actual sobre incluir una opción en la forma de instalar los paquetes de python como en todas las versiones existentes hasta la 3.11.

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 poetry directamente?

danielsp13 commented 1 year ago

@JJ, en https://github.com/danielsp13/SuperCatch/pull/72/commits/30d9ad4654eb98dfa126cf9e1afaeb816582789b he resuelto la cuestión abordada en #78 con lo que me había enlazado en el último comentario, pero después de eso surgió #79, así que considero que lo mejor es dejarlo como lo tenía.

JJ commented 1 year ago

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.

danielsp13 commented 1 year ago

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.

Se ejecutan si los cambios incluye código en el push, como es el caso de https://github.com/danielsp13/SuperCatch/pull/72/commits/e4d9e85a8ad9590d3b7762a5d67caddda284543b (aquí el job job, ¿debo definir una comprobación diferente en GitHub actions?

JJ commented 1 year ago

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.

Se ejecutan si los cambios incluye código en el push, como es el caso de e4d9e85, ¿debo definir una comprobación diferente en GitHub actions?

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.

danielsp13 commented 1 year ago

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.

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.

danielsp13 commented 1 year ago

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.

Se ejecutan si los cambios incluye código en el push, como es el caso de e4d9e85, ¿debo definir una comprobación diferente en GitHub actions?

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.

Vale, los siguientes cambios inlcuirán eso también para que lo pueda ver. Le volveré a mencionar cuando tenga todo listo.

danielsp13 commented 1 year ago

@JJ resuelto. he combinado el uso de caché con una imagen que contiene directamente lo que se necesita.