OCA / l10n-italy

Odoo Italian localization
https://www.odoo-italia.org
GNU Affero General Public License v3.0
147 stars 301 forks source link

[16.0] Dipendenza da python successivo a 3.7 #4305

Open TheMule71 opened 1 month ago

TheMule71 commented 1 month ago

Il nuovo template di repository introdotto con #3767 introduce una dipendenza hard su python 3.10+.

Traceback (most recent call last):
  File "/home/marco/src/odoo/16/OCA/l10n-italy/l10n_it_fatturapa_import_zip/tests/test_import_zip.py", line 92, in test_import_zip
    for invoice, expected_values in zip(
TypeError: zip() takes no keyword arguments

L'argomento strict= di zip è stato introdotto in python 3.10. Se provo ad eliminarlo mi becco:

l10n_it_fatturapa_import_zip/tests/test_import_zip.py:92:49: B905 `zip()` without an explicit `strict=` parameter

La documentazione ufficiale dice che la versione minima di python per Odoo 16 è la 3.7. https://www.odoo.com/documentation/16.0//administration/on_premise/source.html

Che ne pensate?

Io per adesso mi limito a fixare i codice che fallisce sulla versioni precedenti di python quando lo trovo...

SirAionTech commented 1 month ago

Grazie della segnalazione!

Il nuovo template di repository introdotto con #3767 introduce una dipendenza hard su python 3.10+.

Dove? Non trovo modifiche alla versione di Python usata, mi pare fosse 3.10 già da prima:

          - container: ghcr.io/oca/oca-ci/py3.10-odoo16.0:latest
-            makepot: "true"
            name: test with Odoo
          - container: ghcr.io/oca/oca-ci/py3.10-ocb16.0:latest

(da https://github.com/OCA/l10n-italy/pull/3767/files#diff-faff1af3d8ff408964a57b2e475f69a6b7c7b71c9978cccc8f471798caac2c88R38-R40)

La documentazione ufficiale dice che la versione minima di python per Odoo 16 è la 3.7. https://www.odoo.com/documentation/16.0//administration/on_premise/source.html

Avevo già notato un disallineamento tra la versione minima indicata da Odoo e quella usata in OCA, questa è la risposta che ottenni:

TBH, python version compatibility with Odoo is a bit of a mess. It's very hard to get a reliable information about which python versions are really supported. So we look at which python version is used on Odoo runbot and odoo.sh and test with that in OCA.

(da https://github.com/OCA/oca-ci/pull/39#issuecomment-1354628447)

Che ne pensate?

Concordo che in OCA si dovrebbe usare la versione di Python indicata da Odoo stesso, ma non credo sia un cambiamento fattibile sulle versioni esistenti; possiamo farci più caso nelle nuove versioni e magari se lo segnaliamo abbastanza presto si può fare.

Aggiungo che nonostante l'indicazione di Odoo, non userei Python 3.7 perché non è più supportato: image(da https://devguide.python.org/versions/)

aleuffre commented 1 month ago

Credo che in questo caso debba essere OCA, per facilità di implementazione di tuttə, a indicare una versione di python di riferimento. Cosa che tacitamente già fa, perché i test e pre-commit vengono eseguiti su una versione specifica, ma IMO essere più espliciti e direttivi su questa cosa aiuterebbe.

TheMule71 commented 1 month ago

Allora, io localmente uso 3.9. Dovrebbe essere una versione supportata per odoo.

Non posso eseguire i test di l10n_it_fatturapa_import_zip perché nei sorgenti c'è strict=True che è stato introdotto nella 3.10.

Il motivo per cui è stato aggiunto tale argomento è per colpa di un controllo del pre-commit, che indipendentemente dalla versione di python che stai usando non ti fa andare oltre se non aggiungi strict=True.

Sto dicendo che la cosa ha poco senso, a meno che non dichiariamo ufficialmente che il branch 16 di OCA/l10n-italy supporta solo 3.10+. Altrimenti, pre-commit va modificato perché NON faccia quel controllo su versioni di python precedenti alla 3.10. Sennò tanto vale che il primo controllo del pre-commit sia sulla versione di python.

Così come è adesso, pre-commit ti forza letteralmente ad usare una sintassi non valida per la versione di python che stai usando per eseguire pre-commit, il che è un po' un controsenso, pre-commit dovrebbe forzarti ad usare codice corretto, non codice sbagliato.

aleuffre commented 1 month ago

Credo basti scrivere python3.10 (o altra versione) in questa riga.

https://github.com/OCA/l10n-italy/blob/c87827f7ce7d8a4c43903b199f6d670d4d61692e/.pre-commit-config.yaml#L36

Sulla CI di OCA la versione di python è fissa e bloccata (e anche discrepante, ho appena notato, tra test e pre-commit)

https://github.com/OCA/l10n-italy/blob/c87827f7ce7d8a4c43903b199f6d670d4d61692e/.github/workflows/pre-commit.yml#L17-L19

https://github.com/OCA/l10n-italy/blob/c87827f7ce7d8a4c43903b199f6d670d4d61692e/.github/workflows/test.yml#L38-L42

Comunque, sono d'accordo con te sul fatto che questa cosa, che è già di fatto così, andrebbe resa esplicita.