Skip to content

Conversation

@guewen
Copy link
Member

@guewen guewen commented Feb 10, 2021

Port from #284 / #311

When RetryableJobError was raised, any change done by the job was not
rollbacked.

Using raise would throw the exception up to the core and rollback, but
we would have a stack trace in the logs for each try. Calling rollback
manually (rollback also clears the env) hide the tracebacks, however,
when the last try fails, the full traceback is still shown in the logs.

Fixes #261

@guewen
Copy link
Member Author

guewen commented Feb 10, 2021

There is an issue because the job has a date_done date set at the retry

@guewen
Copy link
Member Author

guewen commented Feb 10, 2021

There is an issue because the job has a date_done date set at the retry

Fixed by the last commit, not a new thing.

@guewen
Copy link
Member Author

guewen commented Feb 10, 2021

However, the raise means we have a full stack trace every time, filling the logs. In the case of a retryable error, we don't want the exception stack everytime, only on the last one. I'll see if we can handle the rollback locally.

wpichler and others added 2 commits February 10, 2021 12:31
When RetryableJobError was raised, any change done by the job was not
rollbacked.

Using `raise` would throw the exception up to the core and rollback, but
we would have a stack trace in the logs for each try. Calling rollback
manually (rollback also clears the env) hide the tracebacks, however,
when the last try fails, the full traceback is still shown in the logs.

Fixes #261
When Job.date_done has been set, for instance because the job has been
performed, if the job is set back to pending (e.g. a RetryableJobError
is raised), the date_done is kept.
@guewen guewen force-pushed the port-postpone-retry-rollback branch from d37f40c to b42945f Compare February 10, 2021 11:34
@guewen
Copy link
Member Author

guewen commented Feb 10, 2021

@wpichler @agenterp I modified code to manually rollback / clear the env instead of raising, to prevent cluttering the logs with stack traces. Can you check? (same on #311)

@guewen guewen changed the title [13.0][FIX] Fixed #261. Retryable Job Exception must also throw the excepti… [13.0] Fix missing rollback on retried jobs Feb 10, 2021
@guewen
Copy link
Member Author

guewen commented Mar 15, 2021

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at f707889. 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.

RetryableJobError does not Rollback Transaction

6 participants