-
Notifications
You must be signed in to change notification settings - Fork 21
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
Remove direct python 3.8 dependencies from dagster-cloud-action now that it is EOL #201
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
# Builds ghcr.io/daster-io/dagster-manylinux-builder:* | ||
|
||
# This docker image contains the PEX builder (builder.pex) and is capable of | ||
# This docker image contains the PEX builder (builder.pex) and is capable of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might as well fix the outdated comment: This docker image contains the dagster-cloud package and is ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're copying builder.pex below but dont really use it anymore. I think we should remove it, given new versions of dagster-cloud directly reference the a manylinux tag and not latest. So we will not need to move |
||
# building source only dependencies (sdists) for Python that work with Dagster Cloud Serverless base | ||
# images. | ||
|
||
|
@@ -12,8 +12,8 @@ | |
# | ||
# $ echo $GITHUB-PAT | docker login ghcr.io -u USERNAME --password-stdin | ||
# | ||
# $ docker build . -f src/Dockerfile.dagster-manylinux-builder -t ghcr.io/dagster-io/dagster-manylinux-builder:latest | ||
# $ docker push ghcr.io/dagster-io/dagster-manylinux-builder:latest | ||
# $ docker build . -f src/Dockerfile.dagster-manylinux-builder -t ghcr.io/dagster-io/dagster-manylinux-builder:YOUR_IMAGE_TAG_HERE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we're now publishing these with a tag that matches the base manylinux version used, eg. Should we document that here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we change the tag whenever the underlying image changes, even if the base manylinux version hasn't changed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Otherwise there is no way to downgrade if there is an issue There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah we could push a tag-per-build but there is no versioning system set up for that. |
||
# $ docker push ghcr.io/dagster-io/dagster-manylinux-builder:YOUR_IMAGE_TAG_HERE | ||
|
||
# See dagster-cloud (docker_runner.py) for code that uses this image. | ||
|
||
|
@@ -23,13 +23,13 @@ | |
FROM --platform=linux/amd64 quay.io/pypa/manylinux_2_28_x86_64:latest | ||
|
||
# Add all the relevant Python binaries to the PATH | ||
ENV PATH="/opt/python/cp38-cp38/bin:/opt/python/cp37-cp37m/bin:/opt/python/cp39-cp39/bin:/opt/python/cp310-cp310/bin:$PATH" | ||
ENV PATH="/opt/python/cp38-cp38/bin:/opt/python/cp39-cp39/bin:/opt/python/cp310-cp310/bin:/opt/python/cp311-cp311/bin:/opt/python/cp311-cp312/bin:$PATH" | ||
|
||
# To install unreleased versions, build the wheels and drop them in this directory | ||
COPY wheels /wheels | ||
|
||
# Install dagster-cloud | ||
RUN python3.8 -m pip install dagster-cloud --find-links file:///wheels/ | ||
RUN python3.11 -m pip install dagster-cloud --find-links file:///wheels/ | ||
|
||
COPY generated/gha/builder.pex /builder.pex | ||
|
||
|
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.
probably also want to fix https://github.com/dagster-io/dagster-cloud-action-test/blob/main/templates/deploy.yml#L22