-
-
Notifications
You must be signed in to change notification settings - Fork 312
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_endpoint: cross worker fix #731
[14.0] edi_endpoint: cross worker fix #731
Conversation
0d7cc81
to
f22b493
Compare
909e2cc
to
92c9606
Compare
1b05771
to
7225ab9
Compare
I don't really understand WTF is going on here. I'll try to explain here (also to talk to my GH rubber duck 😁 ).
The error:
It's like the model is not fully registered in the registry at that point. Right now I added [a dummy test](test sync on GH) to call that method in a normal SavepointCase test |
Ok. it fails on SavepointCase too... at least some consistency... 😄 |
Ok, I can't believe it... now it passes 🤔 |
bd0eb11
to
3c80c9a
Compare
I think I've found the reason: declaring again the dependency on the sync mixin messed up the inheritance. |
Former version of `endpoint_route_handler` had a major flaw: routing rule registry was not properly shared across workers forcing us to restart the instance to make sure all envs were inline w/ it. This change adapts edi_endpoint_oca to the new version which contains some refactoring.
3c80c9a
to
49f21dc
Compare
As the main PR has been merged this is required now to make edi_endpoint work. I tested everything again and it works smoothly on my side. I merge here but if anyone spots something wrong/weird just ping me :) /ocabot merge nobump |
On my way to merge this fine PR! |
@@ -18,7 +13,7 @@ class EDIEndpoint(models.Model): | |||
""" | |||
|
|||
_name = "edi.endpoint" | |||
_inherit = "endpoint.mixin" | |||
_inherit = ["endpoint.mixin"] |
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.
Your test issue was coming from there? Should it deserves a comment explaining this, someone that will migrate this later could remove these square brackets.
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.
no no, this is a left over / non necessary change. I added the sync mixin here 1st then I removed it ;)
Congratulations, your PR was merged at b522ce3. Thanks a lot for contributing to OCA. ❤️ |
Replaces #633
Former version of endpoint_route_handler had a major flaw:
routing rule registry was not properly shared across workers
forcing us to restart the instance to make sure all envs were inline w/ it.
This change adapts edi_endpoint_oca to the new version which contains
some refactoring.
Depends on: