-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
[14.0] edi: generate/send chain jobs + add identity exact match based on checksum #796
Conversation
Hi @etobella, |
2a56439
to
4437f44
Compare
Indenty key does not work fine w/ job deps. We'll need this OCA/queue#546 |
4437f44
to
91e7abf
Compare
The recommended way to execute actions on records is to call `action_exchange_*` on
91e7abf
to
e9c1ae8
Compare
Instead of waiting for the cron to pass again and send the file chain the 2 jobs so that it gets sent right after generation.
* catch OSError * add debug log
Prevents having duplicated jobs for the same record as far as possible.
1601216
to
fc0780a
Compare
Try to send out the file as fast as possible once the content is ready.
@etobella @HviorForgeFlow if you could pass by... 🙏 |
hasher = identity_exact_hasher(job_) | ||
# Include files checksum | ||
hasher.update( | ||
str(sorted(job_.recordset.mapped("exchange_filechecksum"))).encode("utf-8") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do yo need to add the file checksum? The function already contains ID of the exchange record and function 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because is the only to way to know if the file is the same.
If by error you try to schedule another job you won't get duplicated jobs unless the content of the file is the same.
In most cases, if you rely only on the cron to process files you'll have the filter done by the check methods that will skip records not in the rate state or not w/ the right values to be generated/sent/received/processed.
However, if you force those checks and try to schedule a job for the a record whereas the file didn't change you shouldn't be able to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might come handy also to compare files w/o reading the whole file (eg: to check if you received the same file twice).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will happen if the file is not generated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, I didn't want to set the approval, I just wanted to mark it 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review 👍
ping @MiquelRForgeFlow
This PR has the |
@etobella good for you or you seen any possible regression on your side? |
I don't see any major regressions from my side. 🤔 I tested with other OCA reports and everything works fine |
/ocabot merge minor |
What a great day to merge this nice PR. Let's do it! |
Congratulations, your PR was merged at 0a8ae3f. Thanks a lot for contributing to OCA. ❤️ |
@simahawk I migrated Just a question, could we change it to use WDYT? |
As long OSError are part of retryable exceptions and HTTPError inherits from OSSError we are catching the new exception * PR introducing that change: OCA#796 * HTTPError implementation: https://github.com/python/cpython/blob/3.12/Lib/urllib/error.py
As long OSError are part of retryable exceptions and HTTPError inherits from OSSError we are catching the new exception * PR introducing that change: OCA#796 * HTTPError implementation: https://github.com/python/cpython/blob/3.12/Lib/urllib/error.py
As long OSError are part of retryable exceptions and HTTPError inherits from OSSError we are catching the new exception * PR introducing that change: OCA#796 * HTTPError implementation: https://github.com/python/cpython/blob/3.12/Lib/urllib/error.py
Depends on:
See atomic commits for details.
Would be nice to use job chain for input too. Not a blocker tho.