Remove direct python 3.8 dependencies from dagster-cloud-action now that it is EOL #201
Conversation
|
Your pull request is automatically being deployed to Dagster Cloud.
|
gibsondan
force-pushed
the
buildmorepex2
branch
2 times, most recently
from
f19ea3b to
0ed5f5a
Compare
| @@ -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.
Might as well fix the outdated comment:
This docker image contains the dagster-cloud package and is ...
There was a problem hiding this comment.
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 latest and (very) old versions of dagster-cloud that use builder.pex will continue to work.
| @@ -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.
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.
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.
Otherwise there is no way to downgrade if there is an issue
There was a problem hiding this comment.
Yeah we could push a tag-per-build but there is no versioning system set up for that.
|
IIUC we dont need a replacement for the "setup 3.8" step because our new build process produces a multi-platform pex that can run with 3.8 or 3.10 or 3.12 and one of these is expected in any github runner. Leaving a note in this PR for context. |
|
Will this be 0.2 or remain 0.1? |
|
I don't see any breaking changes here, so I was going to leave it 0.1 |
| @@ -24,7 +24,7 @@ env: | |||
| jobs: | |||
| dagster_cloud_default_deploy: | |||
| name: Dagster Serverless Deploy | |||
| runs-on: ubuntu-20.04 | |||
There was a problem hiding this comment.
probably also want to fix https://github.com/dagster-io/dagster-cloud-action-test/blob/main/templates/deploy.yml#L22
…hat it is EOL Summary: - Don't install python 3.8 in every serverless build unless that version is what is being used - Use python 3.11 instead of 3.8 in the manylinux builder Test Plan: Create a release and run the action on a serverless deploy ofpython 3.8 through 3.12 Run a docker build locally using the latest manylinux builder
gibsondan
force-pushed
the
buildmorepex2
branch
from
0ed5f5a to
909f747
Compare
Summary:
Test Plan:
Create a release and run the action on a serverless deploy ofpython 3.8 through 3.12
Run a docker build locally using the latest manylinux builder