Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.0] l10n_it_fatturapa: xml doctor for fatturapa #1218

Merged
merged 1 commit into from May 23, 2019
Merged

[10.0] l10n_it_fatturapa: xml doctor for fatturapa #1218

merged 1 commit into from May 23, 2019

Conversation

sherpya
Copy link
Member

@sherpya sherpya commented May 3, 2019

proposal per #1172

it fixes:

  • removes DataOraConsegna, DataOraRitiro if dates are bogus
    i.e. 0001-01-01T00:00:00.000+02:00 where python raises
    OverflowError
  • removes timezone from Date to make pyxb able to compare with
    1-1-1970, it also removes the need of patching pyxb

breaking change:
modules needs to import binding.fatturapa instead of
bindings.fatturapa_v_1_2, this would be usefull also for
new specs

--
Confermo di aver firmato il CLA https://odoo-community.org/page/cla e di aver letto le linee guida su https://odoo-community.org/page/contributing

@sherpya
Copy link
Member Author

sherpya commented May 3, 2019

@eLBati travis ha ricominciato a rompersi su roba che non ci incastra

@eLBati
Copy link
Member

eLBati commented May 6, 2019

travis ha ricominciato a rompersi su roba che non ci incastra

se i problemi non riguardano righe modificate da te, puoi ignorarli

@sherpya
Copy link
Member Author

sherpya commented May 6, 2019

travis ha ricominciato a rompersi su roba che non ci incastra

se i problemi non riguardano righe modificate da te, puoi ignorarli

è abbastanza randomico

@sherpya sherpya changed the title l10n_it_fatturapa: preprocessor for fatturapa in xml l10n_it_fatturapa: xml doctor for fatturapa in xml May 7, 2019
@sherpya sherpya changed the title l10n_it_fatturapa: xml doctor for fatturapa in xml l10n_it_fatturapa: xml doctor for fatturapa May 7, 2019
@sherpya
Copy link
Member Author

sherpya commented May 8, 2019

@eLBati mi sembra ok, ora sistemo il test per controllare il risultato

2019-05-07 21:51:35,903 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed timezone information from date only 2014-12-18
2019-05-07 21:51:35,904 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed timezone information from date only 2015-07-01
2019-05-07 21:51:35,905 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed invalid dateTime 0001-01-01T00:00:00.000+02:00 for DataOraConsegna: date value out of range
2019-05-07 21:51:35,905 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: mandatory element Descrizione was empty, replacing with a dash
2019-05-07 21:51:35,908 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed empty Causale element
2019-05-07 21:51:35,909 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed empty NumeroCivico element
2019-05-07 21:51:36,081 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed timezone information from date only 2014-12-18
2019-05-07 21:51:36,082 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed timezone information from date only 2015-07-01
2019-05-07 21:51:36,083 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed invalid dateTime 0001-01-01T00:00:00.000+02:00 for DataOraConsegna: date value out of range
2019-05-07 21:51:36,083 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: mandatory element Descrizione was empty, replacing with a dash
2019-05-07 21:51:36,086 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed empty Causale element
2019-05-07 21:51:36,087 6969 WARNING openerp_test odoo.addons.l10n_it_fatturapa.bindings.fatturapa: removed empty NumeroCivico element

@eLBati eLBati changed the title l10n_it_fatturapa: xml doctor for fatturapa [10.0] l10n_it_fatturapa: xml doctor for fatturapa May 13, 2019
Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anche

l10n_it_fatturapa/bindings/fatturapa.py:8:1: F403 'from binding import *' used; unable to detect undefined names
l10n_it_fatturapa/bindings/fatturapa.py:42:20: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:44:23: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:49:20: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:51:23: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:57:25: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:58:66: E261 at least two spaces before inline comment
l10n_it_fatturapa/bindings/fatturapa.py:80:23: F405 'CreateFromDocument' may be undefined, or defined from star imports: binding

l10n_it_fatturapa/bindings/fatturapa.py Outdated Show resolved Hide resolved
l10n_it_fatturapa/bindings/fatturapa.py Show resolved Hide resolved
@sherpya
Copy link
Member Author

sherpya commented May 13, 2019

Anche

l10n_it_fatturapa/bindings/fatturapa.py:8:1: F403 'from binding import *' used; unable to detect undefined names
l10n_it_fatturapa/bindings/fatturapa.py:42:20: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:44:23: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:49:20: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:51:23: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:57:25: F405 '_root' may be undefined, or defined from star imports: binding
l10n_it_fatturapa/bindings/fatturapa.py:58:66: E261 at least two spaces before inline comment
l10n_it_fatturapa/bindings/fatturapa.py:80:23: F405 'CreateFromDocument' may be undefined, or defined from star imports: binding

non posso mettere a mano 500 imports :D c'è posso dire a flake di ignorare la cosa? per E261 è una # di refuso

@eLBati
Copy link
Member

eLBati commented May 13, 2019

non posso mettere a mano 500 imports

Come mai 500? Non basterebbero _root e CreateFromDocument?

@sherpya
Copy link
Member Author

sherpya commented May 13, 2019

non posso mettere a mano 500 imports

Come mai 500? Non basterebbero _root e CreateFromDocument?

parlavo del wild import (ho messo ignore), _root è stupido flake non si accorge che lo assegno sotto

@sherpya
Copy link
Member Author

sherpya commented May 13, 2019

si è intrippato travis, da errori differenti.. :(

@eLBati
Copy link
Member

eLBati commented May 13, 2019

Che in locale non ottieni?

@sherpya
Copy link
Member Author

sherpya commented May 13, 2019

Che in locale non ottieni?

In locale ho sempre casino, cmq ho ri-pushato credo che aggiungesse un '\n\n' alle inconsistenze che a qualche test non piaceva

@sherpya
Copy link
Member Author

sherpya commented May 13, 2019

sembra ok ora, almeno la mia roba

Copy link
Member

@eLBati eLBati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per il resto OK per me

Copy link
Member

@SimoRubi SimoRubi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grazie della PR!
Solo qualche commento non bloccante

l10n_it_fatturapa/bindings/README.md Outdated Show resolved Hide resolved
l10n_it_fatturapa/bindings/fatturapa.py Outdated Show resolved Hide resolved
l10n_it_fatturapa/bindings/fatturapa.py Outdated Show resolved Hide resolved
l10n_it_fatturapa/bindings/fatturapa.py Outdated Show resolved Hide resolved
l10n_it_fatturapa/bindings/fatturapa.py Outdated Show resolved Hide resolved
target[path] = mandatory
else:
assert target[path] == mandatory, 'Element already present with ' \
'different minOccurs valus'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'different minOccurs valus'
'different minOccurs value'

Si potrebbe anche aggiungere il path (e volendo anche il text) per uniformarlo agli altri messaggi di warning e poter capire dai log che sta succedendo

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ma è un assert, cmq dimmi cosa scrivo, ce lo metto, non dovrebbe capitare

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

        if target[path] != mandatory:
            msg = "Element having path %s already present with a different minOccurs value" % path
            _logger.warn(msg)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no deve proprio scoppiare se capita :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Va bene, secondo me è utile indicare il path di cosa sta fallendo nei log per capire cosa sta succedendo e dove.
Se usarlo per sollevare eccezione o con un assert decidi tu

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

si ora riproduco aggiungendo un doppione nell'xsd e vedo come viene meglio l'errore

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

credo una cosa simile sia accettabile:
Element //DatiTrasporto/Descrizione is already present with different minOccurs value

it fixes:
 - removes xs:dateTime if bogus and not mandatory
   i.e. 0001-01-01T00:00:00.000+02:00 where python raises
   OverflowError
 - removes timezone from xs:date to make pyxb able to compare with
   1-1-1970, it also removes the need of patching pyxb
 - removes space only strings if not mandatory, else replace with
   a dash

breaking change:
  modules needs to import binding.fatturapa instead of
  bindings.fatturapa_v_1_2, this would be asl useful for
  new specs
@eLBati
Copy link
Member

eLBati commented May 22, 2019

@SimoRubi @sherpya quindi procedo con il merge?

@sherpya
Copy link
Member Author

sherpya commented May 22, 2019

@SimoRubi @sherpya quindi procedo con il merge?

le modifiche richieste sono state effettuate, se non ne avete altre puoi procedere

@eLBati eLBati merged commit 2db2494 into OCA:10.0 May 23, 2019
@sherpya sherpya deleted the xmldoctor branch May 23, 2019 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants