Skip to content
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

Merged
merged 3 commits into from
Aug 7, 2024

Conversation

mdonadoni
Copy link
Member

return response_or_exc

return app
from invenio_app.factory import create_app
Copy link
Member Author

@mdonadoni mdonadoni Jul 2, 2024

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

Copy link
Member Author

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

Copy link
Member Author

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 calls invenio_app.factory.create_app.
  • When running in production mode, uwsgi is used, and the module configured is invenio_app.wsgi:application. This calls invenio_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 with reana_server.factory.create_app.

Copy link
Member Author

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.

Copy link
Member Author

@mdonadoni mdonadoni Jul 10, 2024

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 with invenio_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 in factory.py has been renamed to create_minimal_app, and it is now used in the tests and in generate_openapi_spec.py

Copy link
Member Author

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)

requirements.txt Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
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",
Copy link
Member Author

@mdonadoni mdonadoni Jul 2, 2024

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, ...)

Copy link
Member Author

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

setup.py Outdated Show resolved Hide resolved
@mdonadoni mdonadoni force-pushed the ubuntu-24.04 branch 3 times, most recently from 1dc06b3 to 5a72356 Compare July 4, 2024 09:42
@mdonadoni mdonadoni force-pushed the ubuntu-24.04 branch 2 times, most recently from c2f333d to 11b0522 Compare July 8, 2024 12:03
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 8, 2024
@mdonadoni mdonadoni changed the title wip: upgrade to ubuntu 24.04 build(docker): upgrade base image to ubuntu 24.04 Jul 8, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 9, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 9, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 9, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 9, 2024
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 6 lines in your changes missing coverage. Please review.

Project coverage is 60.46%. Comparing base (c1b4b71) to head (9a7bd8c).
Report is 18 commits behind head on master.

Files with missing lines Patch % Lines
reana_server/ext.py 0.00% 5 Missing ⚠️
reana_server/app.py 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            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     
Files with missing lines Coverage Δ
reana_server/factory.py 100.00% <100.00%> (ø)
reana_server/app.py 0.00% <0.00%> (ø)
reana_server/ext.py 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 10, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 10, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 10, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 22, 2024
@mdonadoni mdonadoni force-pushed the ubuntu-24.04 branch 2 times, most recently from e6e9cbb to c58552e Compare July 22, 2024 15:03
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 23, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 23, 2024
…ub#692)

Fixes Flask warning for the deprecation of the `before_first_request`
decorator.
@mdonadoni mdonadoni marked this pull request as ready for review July 23, 2024 12:28
@mdonadoni
Copy link
Member Author

TODO: check that everything builds and works on ARM64 (e.g. Apple Silicon)

mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 24, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 24, 2024
…ub#692)

Fixes Flask warning for the deprecation of the `before_first_request`
decorator.
@mdonadoni mdonadoni changed the title build(docker): upgrade base image to ubuntu 24.04 build(docker): upgrade to ubuntu 24.04 and Python 3.12 Jul 24, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 31, 2024
mdonadoni added a commit to mdonadoni/reana-server that referenced this pull request Jul 31, 2024
…ub#692)

Fixes Flask warning for the deprecation of the `before_first_request`
decorator.
@tiborsimko tiborsimko merged commit 9a7bd8c into reanahub:master Aug 7, 2024
14 checks passed
@mdonadoni mdonadoni deleted the ubuntu-24.04 branch August 7, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build: upgrade all base docker images to ubuntu 24.04
2 participants