-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[FIX] Request id no longer exists after concurrency rollback #701
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
[FIX] Request id no longer exists after concurrency rollback #701
Conversation
sebalix
left a comment
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.
Minor change, otherwise 👍
| # © 2015 ABF OSIELL <http://osiell.com> | ||
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl). | ||
|
|
||
| from psycopg2.extensions import AsIs |
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.
One line space between third modules and Odoo imports?
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.
Mmm, this is not linted and it's not even in the guidelines. https://github.com/OCA/maintainer-tools/blob/master/CONTRIBUTING.md#imports only explicitely mentions the order of the imports, even if it uses newlines to clarify the grouping in the example. Seems like a cornercase to me, no?
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.
Yes it is, that's why I am not blocking the PR ;) I'm used to do this to clarify the Python dependencies (like the example you linked), and it must be difficult for a quality checker to guess that I think.
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.
In the guidelines recommend us run isort
This lint detect this case and auto-fix.
But it is optional.
If you don't want is goos to us
moylop260
left a comment
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.
LGTM
I didn't know AsIs method to avoid sql injection from table section
Thanks!
|
@StefanRijnhart Do you think this error could occurred on 10.0? To know if there is an interest to include it in #707 |
|
I would be surprised if it did not occur in 10.0, so please have it included there. |
|
Two approvals, is someone available to merge this PR please ? ;) |
Syncing from upstream OCA/server-tools (10.0)
The request id stored on the http request can be rolled back when a concurrency error occurs. Upon retry, the id does not exist anymore.
Related logs look like this: