-
Notifications
You must be signed in to change notification settings - Fork 10
Migrate Quarry to use Poetry for dependency management. #93
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
base: main
Are you sure you want to change the base?
Conversation
Replaced requirements.txt with pyproject.toml/poetry.lock and updated Dockerfile for clean builds. Task: T397332
supertassu
left a comment
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.
The dependency list will need to be pruned to only include direct dependencies and not transient ones to get full advantage of a proper dependency manager. That can be done on this patch or in a follow-up.
More critically, with these changes the development environment fails to even start:
web-1 | Traceback (most recent call last):
web-1 | File "/app/wsgi.py", line 1, in <module>
web-1 | from quarry.web.app import create_app
web-1 | File "/app/quarry/web/app.py", line 1, in <module>
web-1 | from flask import current_app, Flask, render_template, g, Response
web-1 | ModuleNotFoundError: No module named 'flask'
Similarly the tox checks need to be fixed, see the failing CI run. Please fully test your changes locally before sending them for a proper review.
(Finally, for future reference, please follow https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines for your commit messages.)
a998637 to
dca5daf
Compare
|
Hi @supertassu , I’ve updated the workflows to use actions/checkout@v4 with ${{ github.head_repo.full_name }} and confirmed the changes are present in this branch. However, the same issue still persists— it’s still failing at the git checkout step. |
|
@supertassu Can you please review it again, I made the changes which you requested earlier, looking for guidance! |
supertassu
left a comment
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.
Did you test these changes locally? The web container is failing to start for me:
web-1 | Creating virtualenv quarry in /app/.venv
web-1 | usage: virtualenv [--version] [--with-traceback] [-v | -q] [--read-only-app-data] [--app-data APP_DATA] [--reset-app-data] [--upgrade-embed-wheels] [--discovery {builtin}] [-p py] [--try-first-with py_exe]
web-1 | [--creator {builtin,cpython3-posix,venv}] [--seeder {app-data,pip}] [--no-seed] [--activators comma_sep_list] [--clear] [--no-vcs-ignore] [--system-site-packages] [--symlinks | --copies] [--no-download | --download]
web-1 | [--extra-search-dir d [d ...]] [--pip version] [--setuptools version] [--no-pip] [--no-setuptools] [--no-periodic-update] [--symlink-app-data] [--prompt prompt] [-h]
web-1 | dest
web-1 | virtualenv: error: argument dest: the destination . is not write-able at /app
| # To mimic a production runtime, uncomment: | ||
| # entrypoint: ["gunicorn", "-w", "2", "--bind", "0.0.0.0:5000", "wsgi:application"] | ||
| entrypoint: ["python3", "wsgi.py"] | ||
| entrypoint: ["poetry", "run", "python3", "wsgi.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.
These need porting to the Helm manifests as well.
Replaced requirements.txt with pyproject.toml/poetry.lock and updated Dockerfile for clean builds.
Task: T397332