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

ci: Automated Y-Stream Releases #190

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

courtneypacheco
Copy link
Contributor

@courtneypacheco courtneypacheco commented Feb 12, 2025

This dev-doc describes a design we can utilize to automate y-stream release processes for each library within the instructlab GitHub org.

Z-stream releases can use the base logic defined in this dev-doc, but will require additional logic to handle backporting, manual rebasing, etc.

@courtneypacheco courtneypacheco force-pushed the automate-releases branch 7 times, most recently from 5cbc346 to b8f8e11 Compare February 12, 2025 14:32
@courtneypacheco courtneypacheco marked this pull request as ready for review February 12, 2025 14:33

With each release, some library maintainers may want to cap the version of certain dependencies within their `requirements.txt` file as well as specify the desired upper cap for each one.

The list of dependencies to cap should be provided in a file called `automated-release-config.yml`. This configuration file may be expanded in the future to accommodate more configurations as needed. Example:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@courtneypacheco Would this automated-release-config.yml file be per y stream release ? Is there a way we could add a checkpoint to verify in the beginning of the workflow to make sure this file exists and if it does, make sure it exists with the necessary details and format ? We could skip the secondary validation, if the file doesn't exist, which seems to be a possibility based on your workflow diagram.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also would it be a good idea to version this file within the y stream release, if things change on us ? In other words, do we want this file to be immutable once the release process kicks off.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now, per y-stream release.

I definitely do plan on adding a check to see if the file exists. :) If it the file doesn't exist, then the code will resort to built-in default values where applicable.

Since dependency capping is the only "configurable" component right now, the default value for the configurable list of components would be an empty list.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

```yaml
name: Create Y-Stream Release

on:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we want to make this also to be available to run outside of this cron schedule for any reason, by exposing the inputs for the workflow ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, as mentioned near the bottom of the document, there is an option to add your own trigger conditions, like workflow_dispatch. When using workflow_dispatch, users can manually kick off a release process.

The trigger conditions is defined at the Git workflow level, but not at the create-automated-release level.

@kami619
Copy link

kami619 commented Feb 12, 2025

@courtneypacheco Thank you for the PR, I think overall intent is on point and the workflow diagram really lays it out to understand it better. Just few comments, but I get those are nit picks.

@courtneypacheco courtneypacheco changed the title ci: Automated Releases ci: Automated Y-Stream Releases Feb 12, 2025
@courtneypacheco courtneypacheco force-pushed the automate-releases branch 2 times, most recently from d034301 to d807e31 Compare February 12, 2025 16:33
Signed-off-by: Courtney Pacheco <6019922+courtneypacheco@users.noreply.github.com>
@ktdreyer
Copy link

Looks good to me.

This is not a blocker, just a thought. enabled: true looks superfluous to me - like if a developer wants to disable dependency_caps, they could specify an empty dict, or delete the dependency_caps section altogether. Or if you want to keep it that feature, maybe we could default enabled: true so users don't have to copy that line around to all repos.

Either way, great feature.

@courtneypacheco courtneypacheco requested a review from a team February 17, 2025 15:18
Comment on lines +11 to +14
3. Optionally trigger an E2E test against that pull request,
4. Wait for all pull request CI checks to complete,
5. Manually request two maintainers to approve the pull request, and
6. Manually create a release from the GitHub UI using that new branch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel the commas and "and" aren't really necessary in a numbered list


### Overview of Y-Stream (Minor) Release Automation

Y-stream (minor) releases have historically been handled differently from Z-stream releases. Z-stream releases oftentimes involve backports for bugfixes and may require manual code rebasing to get those backports merged into the appropriate existing release branch. Therefore, we can think of Y-stream release logic as the "basis" for Z-stream release logic, which takes the Y-stream logic and builds upon it to account for backporting and other desirable actions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Y-stream (minor) releases have historically been handled differently from Z-stream releases. Z-stream releases oftentimes involve backports for bugfixes and may require manual code rebasing to get those backports merged into the appropriate existing release branch. Therefore, we can think of Y-stream release logic as the "basis" for Z-stream release logic, which takes the Y-stream logic and builds upon it to account for backporting and other desirable actions.
Y-stream (minor) releases have historically been handled differently from Z-stream releases. Z-stream releases often times involve backports for bugfixes and may require manual code rebasing to get those backports merged into the appropriate existing release branch. Therefore, we can think of Y-stream release logic as the "basis" for Z-stream release logic, which takes the Y-stream logic and builds upon it to account for backporting and other desirable actions.


### Configurable Components

As mentioned above, there are configurable components within this release process automation. The diagram in the next section references two configurable components:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
As mentioned above, there are configurable components within this release process automation. The diagram in the next section references two configurable components:
As mentioned above, there are configurable components within this release process automation. The diagram in the next section references two configurable components

Comment on lines +70 to +73
packages:
instructlab-sdg: "+0.1.0"
instructlab-eval: "+0.2.0"
instructlab-training: "+1.0.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion here, lmk what you think

Suggested change
packages:
instructlab-sdg: "+0.1.0"
instructlab-eval: "+0.2.0"
instructlab-training: "+1.0.0"
packages:
- name: instructlab-sdg
greater_than: 0.1.0
- name: instructlab-eval
greater_than: 0.2.0
- name: instructlab-training
greater_than: 1.0.0

You could have similar fields equals or less_than when applicable

schedule:
- cron: '0 7 * * 1 '

# Allow manual dispatch, too
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Allow manual dispatch, too
# Allow manual dispatch too

Copy link

@booxter booxter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have my reservations about automation of capping. We should just stop capping.

For example, for a typical y-stream release, a maintainer has to:

1. Manually create a new release branch -- e.g., `release-0.y.0`,
2. Manually create a pull request against `release-0.y.0` to cap the versions on some of the dependencies defined in their library's `requirements.txt` file,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this requirement to cap coming from? Aren't we asked to not cap?

Only apply "caps" to dependencies (using <) when that dependency has established a pattern of producing new releases with breaking changes.

Without this, steps 3-5 are no longer necessary.

Copy link
Contributor Author

@courtneypacheco courtneypacheco Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have been capping certain dependencies in the core repo since October 2024: https://github.com/instructlab/instructlab/pulls?q=is%3Apr+%22deps%3A+cap%22+is%3Aclosed+

To elaborate, we have been capping our own InstructLab library dependencies to ensure that if we create a new release from the core repo, that new release won't automatically consume potential breaking changes from one of our own libraries.

If we don't cap certain dependencies, then we'll need to address those breaking changes in one or more pull requests and follow up by publishing a Z-stream release so that end users can consume the fixes. So I think for end users in particular, it can be extremely frustrating to pull the latest InstructLab release from 1-7 days ago, only to find out that the InstructLab release doesn't work because it's pulling an incompatible package.

Copy link
Contributor Author

@courtneypacheco courtneypacheco Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, to be clear, the "automatic capping" feature is not required to be used by anybody. Maintainers can ignore the feature if desired. And since the automation described here will create a pull request that someone has to manually approve and review, the dependency capping changes will not be merged automatically. 😃

- cron: '30 1 1,15 * *' # Triggers at 1:30am UTC every 2 weeks on the 1st and the 15th day of each month
```

#### Custom List of Dependencies to Cap
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above. Capping is not something we should automate. It's an exceptional situation, not a matter of course.

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.

6 participants