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

[14.0] edi_oca: fix/imp jobs #855

Merged
merged 5 commits into from
Nov 23, 2023
Merged

Conversation

simahawk
Copy link
Contributor

@simahawk simahawk commented Nov 7, 2023

@simahawk simahawk changed the title 14 edi fix jobs [14.0] edi_oca: fix/imp jobs Nov 7, 2023
@HviorForgeFlow
Copy link
Member

After looking into the comments here: OCA/queue#573 (comment)

What about adding some json field that handles that kind of technical related records. I'm not sure if this will be feasible.

adding some kind of json with model.name key and array of ids like {"queue.job": [1,2,3]} maybe if only queue jobs are necessary it can be simplified.

Just guessing, WDYT? @etobella @simahawk

@simahawk
Copy link
Contributor Author

After looking into the comments here: OCA/queue#573 (comment)

What about adding some json field that handles that kind of technical related records. I'm not sure if this will be feasible.

adding some kind of json with model.name key and array of ids like {"queue.job": [1,2,3]} maybe if only queue jobs are necessary it can be simplified.

Just guessing, WDYT? @etobella @simahawk

Yeah, I don't know yet.... For the moment I will drop this part. If you come up w/ an idea meanwhile, feel free to take it over! 😉

@simahawk simahawk marked this pull request as ready for review November 21, 2023 10:22
@simahawk
Copy link
Contributor Author

Tests are failing because of #864. Fix on #865

@simahawk
Copy link
Contributor Author

@HviorForgeFlow would you mind to drop a review pls?

Send jobs might fail due to an external service being not responsive.
If the job is simply failed, a new one will be spawned and might encour in the same error again,
possibly leading to an high number of duplicated jobs for the same record.

Yet, when a RetryableJobError is raised, the job will be set back into pending state
and will be nicely retried based on jobs configuration.
When actions are automated, is nice to see that data has been generated for an exchange on the related record.
exchange_generate now returns status string, not the output.
@simahawk
Copy link
Contributor Author

Picked
edi: add generate_ok message from #796

@simahawk
Copy link
Contributor Author

@HviorForgeFlow @etobella gentle ping

Copy link
Member

@etobella etobella left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@bosd bosd left a comment

Choose a reason for hiding this comment

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

LGTM

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@etobella
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-855-by-etobella-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit d3a8b93 into OCA:14.0 Nov 23, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 32eea26. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants