- 
                Notifications
    
You must be signed in to change notification settings  - Fork 551
 
Adjust release process after quickstart changes #4104
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: develop
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -77,7 +77,6 @@ steps: | |
| --platform linux/amd64 \ | ||
| --build-arg BASE_IMAGE=$$USERNAME/prepare-release:base-${_ZENML_NEW_VERSION} \ | ||
| --build-arg CLOUD_PROVIDER=gcp \ | ||
| --build-arg ZENML_BRANCH=${_ZENML_BRANCH} \ | ||
| 
         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. I am confused about all this. Maybe you can clear it for me. I see now that we don't even build any quickstart images for the release (which I like). But if that's the case, why do we even need to build them now for testing the release? Isn't ok for us to use the base image to test? Previously, different quickstarts required a different "pre-specified" list of requirements to run the quickstart. As far as I can see, this is no longer a concern of the new one. 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. These image still installed the stack requirements in there, but I don't even understand why that was necessary back then. I removed all of that now though.  | 
||
| -f docker/zenml-quickstart-dev.Dockerfile \ | ||
| -t $$USERNAME/prepare-release:quickstart-gcp-${_ZENML_NEW_VERSION} | ||
| 
     | 
||
| 
        
          
        
         | 
    @@ -97,7 +96,6 @@ steps: | |
| --platform linux/amd64 \ | ||
| --build-arg BASE_IMAGE=$$USERNAME/prepare-release:base-${_ZENML_NEW_VERSION} \ | ||
| --build-arg CLOUD_PROVIDER=aws \ | ||
| --build-arg ZENML_BRANCH=${_ZENML_BRANCH} \ | ||
| -f docker/zenml-quickstart-dev.Dockerfile \ | ||
| -t $$USERNAME/prepare-release:quickstart-aws-${_ZENML_NEW_VERSION} | ||
| id: build-quickstart-aws | ||
| 
        
          
        
         | 
    @@ -116,7 +114,6 @@ steps: | |
| --platform linux/amd64 \ | ||
| --build-arg BASE_IMAGE=$$USERNAME/prepare-release:base-${_ZENML_NEW_VERSION} \ | ||
| --build-arg CLOUD_PROVIDER=azure \ | ||
| --build-arg ZENML_BRANCH=${_ZENML_BRANCH} \ | ||
| -f docker/zenml-quickstart-dev.Dockerfile \ | ||
| -t $$USERNAME/prepare-release:quickstart-azure-${_ZENML_NEW_VERSION} | ||
| id: build-quickstart-azure | ||
| 
          
            
          
           | 
    ||
This file was deleted.
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.
As far as I can see this used to generate
examples/quickstart/requirements_{aws,azure,gcp}.txtfiles for the quickstart testing script. Since this is now disabled (which was the right call I think), wouldn't theqs_run_release_flowfail?It has a check:
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.
Same script also looks for config files that do not exist.
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 installation seems a bit weird as well. The runner starts, checks out to the branch, already install ZenML once, passes the branch name to the test script, the test script manually replaces the hard-coded zenml requirement (which is the only requirement), pip installs zenml throught the
requirements.txtfile again and only then runs the example. This seems like a very complicated and redundant process.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.
Yep, this whole thing has been a mess. I removed as much of it as I could now.