-
Notifications
You must be signed in to change notification settings - Fork 13
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
[Issue #3066] Expose analytics CLI entry point in Docker #3085
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The db-migrate entry point is already exposed through a CLI sub-command: `analytics etl db_migrate`
poetry install --no-root installs the application in an editable mode building the wheel then installing it gets a stable version of the app
widal001
commented
Dec 4, 2024
widal001
commented
Dec 4, 2024
coilysiren
reviewed
Dec 4, 2024
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.
Almost there!
coilysiren
approved these changes
Dec 4, 2024
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.
🚀 (even just what you have here was important to figure out)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Exposes the
analytics
CLI entry point for the Docker image that allows us to rundocker run <image> etl db_migrate
as part of the CI/CD pipeline.Fixes #3066
Time to review: 2 mins
Changes proposed
poetry run pip install
to use--no-cache-dir
and use a wildcard for thedist/*.whl
so that users don't have to change that line if we bump the version.Context for reviewers
To prove that the entry point is available to run from the Docker container (without
poetry run
) do the following steps:make release-build
from the root of the/analytics/
sub-directorysha256:<image-hash>
from the resulting docker logs:docker run <image-hash> analytics etl --help
and it should print out:Note: Trying to run the actual DB migration with this method will fail because the migration depends on interacting with a live Postgres instance, which is what we use
docker compose
for but the currentdocker-compose.yml
has the target set todev
based on this Slack thread testing it withdocker compose run
may not be necessary.Additional information
There are two main strategies for installing the package and exposing the CLI entry point:
poetry install --only-root
Not chosen because it installs an editable version of the python package (equivalent to doingpip install . -e
) which is usually reserved for developmentpoetry build ... && poetry run pip install ...
Chosen because it first builds a stable version of the package and installs it