-
Notifications
You must be signed in to change notification settings - Fork 66
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
[ENG-7411] poetry and assorted cleanup #852
base: feature/share-cleanupgrade-2025
Are you sure you want to change the base?
[ENG-7411] poetry and assorted cleanup #852
Conversation
a3de9f3
to
07840eb
Compare
This reverts commit 5776adc.
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.
Looks good to me! I had a couple of small questions, but neither is a dealbreaker.
Cheers,
@felliott
### Dev | ||
FROM app AS dev | ||
|
||
RUN pip install --no-cache-dir -c /code/constraints.txt -r /code/dev-requirements.txt | ||
RUN $POETRY_HOME/bin/poetry install --compile --only dev |
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.
Looks good. Docker build had a few nitpicks, but these aren't dealbreakers:
3 warnings found (use docker --debug to expand):
- FromAsCasing: 'as' and 'FROM' keywords' casing do not match (line 1)
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 60)
- LegacyKeyValueFormat: "ENV key=value" should be used instead of legacy "ENV key value" format (line 61)
@@ -54,32 +48,28 @@ jobs: | |||
- name: install non-py dependencies | |||
run: sudo apt-get update && sudo apt-get install -y libxml2-dev libxslt1-dev libpq-dev git gcc | |||
|
|||
- name: set up python${{ matrix.python-version }} | |||
- name: Install poetry | |||
run: pipx install poetry |
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.
Should this be pinned to the same version as the Dockerfile (2.1.1)? I don't have a strong opinion. It might be worth to leave unpinned as a canary.
poetry
to install/manage dependenciesrequirements.txt
,dev-requirements.txt
,constraints.txt
pyproject.toml
poetry.lock
Dockerfile
anddocker-compose.yml
to usepoetry
sharev2_elastic5
index strategy)trove_indexcard_flats
index strategy