Skip to content
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

[FAST_BUILD] Introduce fast build #2014

Merged
merged 22 commits into from
Nov 4, 2023
Merged

Conversation

mathbunnyru
Copy link
Member

Describe your changes

Issue ticket if applicable

Checklist (especially for first-time contributors)

  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests
  • I will try not to use force-push to make the review process easier for reviewers
  • I have updated the documentation for significant changes

@mathbunnyru
Copy link
Member Author

This approach allows us to test the most crucial changes in the CI in a fast way.
We need to build 2 images because we really want to check the upload/download part because it might be broken as well.

As I'm not sure this is the best approach, and it adds some noise to the CI graph and the workflow file is bigger, I would like to hear some ideas, maybe there is a better approach.

@mathbunnyru
Copy link
Member Author

If it's a PR, then everything depends on whether the PR title has a [FAST_BUILD] substring.
During the push to the main branch a github.event.pull_request.title property is empty (I hope), so we will always run the full build (which totally makes sense).

@mathbunnyru
Copy link
Member Author

I don't understand what's going on 😢

@mathbunnyru
Copy link
Member Author

mathbunnyru commented Oct 31, 2023

Finally got it working. Today I learned GitHub env variables are strings, not some kind of GitHub magic, so, the "false" string evaluates to true (if then later used as a condition).

What's cool (I suppose it's secure enough, but who knows), is that GitHub evaluates the expression after replacing env variables with values.
https://github.com/jupyter/docker-stacks/actions/runs/6712091950/job/18241119289

@mathbunnyru
Copy link
Member Author

@yuvipanda you might find this feature useful - it allows to significantly reduce build time in PRs.
You need to add [FAST_BUILD] to the PR name and then only 2 root images will be built (and tested).
I think most of CI/infrastructure/start-script PRs will benefit from this change (and I wish I had thought of this feature a few years ago).

Please, review, if you have some time 🙂

@mathbunnyru
Copy link
Member Author

I am merging this. I want to introduce new tests for run-hooks before we merge #2007.

@mathbunnyru mathbunnyru merged commit 9df3959 into jupyter:main Nov 4, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant