-
-
Notifications
You must be signed in to change notification settings - Fork 279
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
[17.0][MIG] fastapi: Migration to 17.0 #409
base: 17.0
Are you sure you want to change the base?
Conversation
The check failed because of the issue #141747 |
Thank you, Mr. Chien, for your contribution. It works for me. |
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.
@nguyenminhchien Thank you for the migration. Con you remove the dependency on odoo-addon-endpoint-route-handler @ git+https://github.com/OCA/web-api.git@refs/pull/31/head#subdirectory=endpoint_route_handler
This one is merged.
bca16d5
to
db8104d
Compare
Removed, thanks. |
/ocabot migration fastapi |
@lmignon @sbidoul it seems that Odoo 17 has issues with the addons having a 16.0.x.y.z version, even if they are installable=False cf. https://github.com/OCA/rest-framework/actions/runs/7634588421/job/20798693591?pr=409#step:7:74 Is this something you prefer reporting as an issue to Odoo or handle in the test scripts or process with a commit updating the versions of the uninstallable addons in the repo? |
An issue is open on odoo regarding this problem odoo/odoo#141747 |
After an update of Odoo the following error is raised on the fastapi module
@api.model
def _eval_context(self):
ctx = super()._eval_context()
if "authenticated_partner_id" in self.env.context:
ctx["authenticated_partner_id"] = self.env.context[
"authenticated_partner_id"
]
return ctx Should be changed to this @api.model
def _eval_context(self):
ctx = super()._eval_context()
ctx["authenticated_partner_id"] = self.env.context.get("authenticated_partner_id")
return ctx |
db8104d
to
f79a73b
Compare
@nguyenminhchien this pull request is now needed too #416 |
114895e
to
956c1a3
Compare
I picked this one. |
/ocabot merge nobump |
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 only
Hey, thanks for contributing! Proceeding to merge this for you. |
/ocabot merge nobump |
On my way to merge this fine PR! |
956c1a3
to
c231e78
Compare
Hi @dreispt could you please try again your merge bot command? I believe it wasn't going through previously because of an old pending runboat build, but this has now been fixed thanks to Chien's rebase |
This PR has the |
/ocabot merge nobump |
On my way to merge this fine PR! |
Yikes, wrong hypothesis 😅 Hi @sbidoul the 17.0 branch of this repo is based on 16.0 and some manual changes Do you think it could now be initialized "normally" through copier? |
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.
A couple of comments "en passant".
There are a few missing commits from the 16.0 branch. Would be nice to pick them up before merging.
I'll look at why the bot does not merge.
fastapi/__manifest__.py
Outdated
@@ -30,5 +30,5 @@ | |||
] | |||
}, | |||
"development_status": "Beta", | |||
"installable": False, | |||
# "installable": False, |
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.
# "installable": False, | |
"installable": True, |
Or remove the line.
fastapi/schemas.py
Outdated
limit: Optional[int] = None # noqa: UP007 | ||
offset: Optional[int] = None # noqa: UP007 |
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.
limit: Optional[int] = None # noqa: UP007 | |
offset: Optional[int] = None # noqa: UP007 | |
limit: int | None | |
offset: int | None |
Would this work?
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 indeed
Hm, weird. Trying again. /ocabot merge nobump |
This PR looks fantastic, let's merge it! |
If you pass an override of the authenticated_partner_impl to the _create_test_client method, don't override it by a default one
This is to ensure that `retrying` in `service/model.py` does not try to flush.
This reverts commit 8be7fbb.
Currently translated at 100.0% (41 of 41 strings) Translation: rest-framework-16.0/rest-framework-16.0-fastapi Translate-URL: https://translation.odoo-community.org/projects/rest-framework-16-0/rest-framework-16-0-fastapi/it/
…en evaluating ir rules This fix is needed since a modificiation of ir.rule that checks the domain when creating/modifying ir rules The solution is to set authenticate_partner_id to False when it is not present in context
From odoo/odoo@cb1d057, the orignal werkzeug request is wrapped in a dedicated class to keep under control the attributes developers use. This change add code to make the fastapi addon working with version including this last change and prior version refs OCA#414
Ah I understand #409 (comment) now. The base branch CI was sill configured for 16.0. I ran |
91e9647
to
a539a1a
Compare
a539a1a
to
564b24e
Compare
Ref: BSRD-708
Depends on: