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.

Sign up for GitHub

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

Jump to bottom

Introduce FastAPI #2745

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Introduce FastAPI #2745

wants to merge 4 commits into from

Conversation

lbarcziova
Copy link
Member

@lbarcziova lbarcziova commented Mar 10, 2025 •
edited
Loading

Merge after #2749 (needed for tests to pass)

Fixes #2715

To discuss:

  • which ASGI server to use: currently I played with this with uvicorn, which is simple and lightweight, it doesn’t have http2 support though (for that we could use e.g. hypercorn), you can see some info on the alternatives here
    • after reading few articles, I went with uvicorn for local development and gunicorn + uvicorn workers for prod/stg, as it is supposed to be more suitable for prod envs (scalable, robust)
  • Flask runs under /api and is hardcoded as WSGI, so I couldn’t make it work so that new endpoints are under /api/v1, for now I put them to /v1, wdyt? (redoc and swagger are also on /v1/redoc and /v1/docs)

TODO:

  • tests
  • rename of run_httpd.sh and occurrences
  • polish the code, FastAPI settings, look into security_and_https_middleware more

RELEASE NOTES BEGIN

N/A

RELEASE NOTES END

@softwarefactory-project-zuul softwarefactory-project-zuul

This comment was marked as outdated.

Sign in to view
@lbarcziova lbarcziova self-assigned this Mar 10, 2025
@softwarefactory-project-zuul softwarefactory-project-zuul

This comment was marked as outdated.

Sign in to view
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/054bcebc86ed4dd99528f8f7156b7438

✔️ pre-commit SUCCESS in 1m 50s
❌ packit-service-tests FAILURE in 1m 17s

Venefilyn
Venefilyn reviewed Mar 11, 2025
View reviewed changes
Copy link
Contributor

@Venefilyn Venefilyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a good start! I think going with /v1 etc. instead of /api/v1 is fine. Would API documentation go to /v1/docs or /docs?

packit_service/service/app.py
Comment on lines +149 to +161
# Swagger and ReDoc don't work without these
"style-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; "
"script-src 'self' 'unsafe-inline' https://cdn.jsdelivr.net; "
Copy link
Contributor

@Venefilyn Venefilyn Mar 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something we should track in a separate issue. If auth ever becomes something Packit implements this might then be a security threat - especially with JS scripts

@lbarcziova
Copy link
Member Author

Would API documentation go to /v1/docs or /docs?

Currently it goes under /docs, but this can be easily changed.

@Venefilyn
Copy link
Contributor

Would API documentation go to /v1/docs or /docs?

Currently it goes under /docs, but this can be easily changed.

Ah okay. Yeah I'd say put it under the specific api version due to using versioning

majamassarini
majamassarini reviewed Mar 12, 2025
View reviewed changes
@lbarcziova lbarcziova marked this pull request as ready for review March 13, 2025 05:49
@lbarcziova lbarcziova requested a review from a team as a code owner March 13, 2025 05:49
@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

Build failed.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/84a877feca1f491da40b948b454e0422

✔️ pre-commit SUCCESS in 1m 53s
❌ packit-service-tests FAILURE in 1m 19s

@lbarcziova lbarcziova mentioned this pull request Mar 13, 2025
Merged
softwarefactory-project-zuul bot added a commit that referenced this pull request Mar 13, 2025
@softwarefactory-project-zuul
Add dependencies for FastAPI changes (#2749)
25d04b4 
Add dependencies for FastAPI changes

Related to #2715
Needed for #2745

Reviewed-by: Nikola Forró
@lbarcziova
Switch to FastAPI + Gunicorn+Uvicorn, run Flask as hardcoded WSGI
ef161a0 
Introduce FastAPI running with Uvicorn (+Gunicorn in prod/stg), keep old Flask endpoints under /api hardcoded as WSGI.
New endpoints will come under /v1, Swagger is at /docs and Redoc under /redoc.
@lbarcziova
Introduce FastAPI healthz and system endpoints
826d5cc
@lbarcziova
Reflect FastAPI changes in tests
0ee44f8
@lbarcziova
Rename run_httpd.sh and occurences
5cd7aeb 
We no longer run httpd.
majamassarini
majamassarini approved these changes Mar 13, 2025
View reviewed changes
Copy link
Member

@majamassarini majamassarini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should land this on stg soon, because now our stg service instance is down.

Thanks!

@lbarcziova
Copy link
Member Author

recheck

@softwarefactory-project-zuul softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.
https://softwarefactory-project.io/zuul/t/packit-service/buildset/4bac654cc0444a3693ce38303101f7db

✔️ pre-commit SUCCESS in 1m 56s
✔️ packit-service-tests SUCCESS in 2m 48s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Venefilyn Venefilyn Venefilyn left review comments

@nforro nforro nforro left review comments

@majamassarini majamassarini majamassarini approved these changes

Assignees

@lbarcziova lbarcziova

Labels
None yet
Projects
Packit Kanban Board
Status: in-review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Introduce FastAPI alongside Flask
4 participants
@lbarcziova @Venefilyn @majamassarini @nforro