-
Notifications
You must be signed in to change notification settings - Fork 38
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
build(docker): upgrade to ubuntu 24.04 and Python 3.12 #692
Conversation
reana_server/factory.py
Outdated
return response_or_exc | ||
|
||
return app | ||
from invenio_app.factory import create_app |
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.
TODO: find out whether using invenio's create_app
has unwanted side-effects, but as far as I can tell the previous create_app
function was only used in tests, and invenio's create_app
was already used when deploying REANA
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.
This change breaks the generation of the OpenAPI specification file.
Also let's check which invenio endpoints are exposed, even more so after adding invenio-access
to the dependencies of REANA. Let's also double check that invenio-access
is really needed
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.
REANA is technically a Flask extension (and not a Flask app), as the main Flask app is created by Invenio. This is how ext.py
/factory.py
worked before this PR set:
- When running in debug mode,
invenio run ...
is invoked. This callsinvenio_app.factory.create_app
. - When running in production mode,
uwsgi
is used, and the module configured isinvenio_app.wsgi:application
. This callsinvenio_app.factory.create_app
. - When running the tests,
reana_server.factory.create_app
is called. This a manually created Flask app with some of Invenio modules loaded. - When running
generate_openapi_spec.py
, the app is created withreana_server.factory.create_app
.
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.
Keeping the previous create_app for tests and for the generate_openapi_spec.py
script. Added some comments to clarify the difference between REANA's create_app and Invenio's create_app.
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.
I forgot the case of flask reana-admin
commands. To also make this work, I have made the following changes:
- The app in
app.py
is created withinvenio_app.factory.create_app
. This is the app that Flask autodetects, so we probably want this to be the same as the production app - The factory method
create_app
infactory.py
has been renamed tocreate_minimal_app
, and it is now used in the tests and ingenerate_openapi_spec.py
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.
I have also checked the endpoints exposed by Invenio before/after the upgrade:
- For
/api
nothing changes - For
/oauth
nothing changes - There are additional endpoints coming from
invenio-access
, but these are not exposed as the ingress only forwards/api
and/oauth
requrests to reana-server (source)
setup.py
Outdated
"invenio-base>=1.2.0,<1.3.0", | ||
"invenio-cache>=1.1.0,<1.2.0", | ||
"invenio-config>=1.0.3,<1.1.0", | ||
"invenio-app>=1.3.0", |
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.
TODO: test that invenio upgrade does not cause issues (e.g. test migrations, login, SSO, ...)
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.
SSO login and migration worked well on DEV
1dc06b3
to
5a72356
Compare
c2f333d
to
11b0522
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #692 +/- ##
==========================================
+ Coverage 60.14% 60.46% +0.31%
==========================================
Files 32 32
Lines 3317 3341 +24
==========================================
+ Hits 1995 2020 +25
+ Misses 1322 1321 -1
|
e6e9cbb
to
c58552e
Compare
…ub#692) Fixes Flask warning for the deprecation of the `before_first_request` decorator.
TODO: check that everything builds and works on ARM64 (e.g. Apple Silicon) |
…ub#692) Fixes Flask warning for the deprecation of the `before_first_request` decorator.
…ub#692) Fixes Flask warning for the deprecation of the `before_first_request` decorator.
…ub#692) Fixes Flask warning for the deprecation of the `before_first_request` decorator.
Closes reanahub/reana#808