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

feat: automated releases via github action #5342

Closed

Conversation

psychedelicious
Copy link
Collaborator

@psychedelicious psychedelicious commented Dec 23, 2023

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update
  • Community Node Submission

Have you discussed this change with the InvokeAI team?

  • Yes
  • No, because:

Have you updated all relevant documentation?

  • Yes
  • No

Description

This PR introduces a new, hands-off release process, which is handled almost entirely by GitHub actions. The only part that is not yet in an action is creating the GitHub release itself, but there are some notes suggesting a path to that (shouldn't be too hard).

This PR was motivated by some hiccups we've seen during releases:

  • Missing frontend builds
  • Complicated build process, run locally and subject to platform-specific quirks
  • PyPI release action that could result in a different wheel being published to PyPI than what is released in the installer

See docs/RELEASE.md for documentation.


feat: automated releases via github action

  • Restructure & update code check workflows
  • Add release workflow to handle checks/tests, build and publish to PyPI
  • Add docs/RELEASE.md explaining the workflow & process
  • create_installer.sh: Update to work with the release workflow
  • create_installer.sh & tag_release.sh: Fix the ANSI escape codes for macOS
  • tag_release.sh: Add check for python binary name
  • tag_release.sh: Print git remote -v output
  • tag_release.sh: Fix error when deleting nonexistant tags

QA Instructions, Screenshots, Recordings

This has been extensively tested on my fork and works great. Because the sensitive publish steps are protected in multiple ways, I think it's safe to merge and test on origin.

Merge Plan

This PR needs review from @lstein. I'd also like to get reviews from @ebr and @brandonrising .

I'll defer to @lstein for whether or not to try this new process out for 3.5.0 or wait for the next release.

We can add up to 6 contributors to the list of approvers for the pypi and testpypi environments. Probably:

Added/updated tests?

  • Yes
  • No : No code changes here

@hipsterusername
Copy link
Member

Awesome - 👏

Copy link
Member

@ebr ebr left a comment

Choose a reason for hiding this comment

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

Fantastic! 👏 This has been long overdue.
Left a few comments that are mostly nitpicks. There will probably be more kinks to work out in github actions once this is merged, but we won't know until the process gets some exercise.

@lstein
Copy link
Collaborator

lstein commented Dec 28, 2023

Sorry, I've been working on the model installer pretty intensively over the past few days. Will give this a review on Thursday.

Copy link
Collaborator

@lstein lstein left a comment

Choose a reason for hiding this comment

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

Looks like this has been picked over very carefully already. I scanned through and couldn't find anything that looked suspicious, so I'm giving it my go ahead. Impressive work!

@psychedelicious
Copy link
Collaborator Author

@lstein @hipsterusername I think this will need to be merged with the big red button bc it changes workflows.

And then the repo settings will need to be updated so that PRs require these three workflows to succeed:

  • On change: run check-frontend
  • On change: run check-python
  • On change: run pytest

Otherwise this is good to go, and we can test it with a dev/alpha release (that we don't publish)

@hipsterusername
Copy link
Member

this is great. going to say we hold off on this until after 3.6 is out

@hipsterusername
Copy link
Member

Want to merge this but going to wait until we have a few folks ready to coordinate the other changes needed for this to succeed

@psychedelicious
Copy link
Collaborator Author

Yeah we should coordinate. I'd like to find a half hour to merge this and do a dev release to pypi on a call with @lstein. Just in case we need to do pypi surgery.

Just had a thought - does the updater only offer to update to RC versions? If we release a dev/nightly version to pypi will users be prompted to update?

Oooh. Nightlies :) that sounds cool.

@Millu
Copy link
Contributor

Millu commented Jan 16, 2024

Just had a thought - does the updater only offer to update to RC versions? If we release a dev/nightly version to pypi will users be prompted to update?

The updater right now only looks for RCs and the latest release, but could be easily updated for nightlies

@psychedelicious
Copy link
Collaborator Author

psychedelicious commented Jan 16, 2024

The updater logic currently only differentiates between RC and non-RC.

This means alpha, dev, and post releases are all considered non-RC releases, and if we do an alpha or dev release, a users running the updater at the right moment will see it as a stable release and download it.

We'll need to do something like this to split up the PyPI versions: https://gist.github.com/psychedelicious/d589c53284f88cad3ea57f320b21687c

@github-actions github-actions bot added documentation Improvements or additions to documentation Root installer PRs that change the installer CICD labels Jan 31, 2024
@psychedelicious
Copy link
Collaborator Author

I've updated this branch.

Post-merge testing:
Testing:

  • Manually dispatch the Release workflow. Confirm it builds a working wheel.
  • Create a PR that changes frontend files and should fail a lint check.
  • Create a PR that changes frontend files and should pass a lint check.
  • Create a PR that changes python files and should fail a lint check.
    • It should also trigger the pytest workflow
  • Create a PR that changes python files and should pass a lint check.
    • It should also trigger the pytest workflow
  • Create a branch that tags a new release. Use the make tag-release command to tag the release accordingly. This should kick off the Release workflow, running all checks. It will pause before anything is published.
    • If all goes well, we can authorize only the testpypi publish step to publish the release to testpypi.
    • Test the testpypi release (instructions in RELEASE.md)

If all of these steps work correctly, theoretically we should be ready for a real release when the time comes.

@psychedelicious psychedelicious force-pushed the feat/release-workflow branch 2 times, most recently from 48c38a3 to 9a3870a Compare February 9, 2024 04:35
@psychedelicious
Copy link
Collaborator Author

I've updated this PR to work with the recent changes to the install process.

- Restructure & update code check workflows
- Add release workflow to handle checks/tests, build and publish to PyPI
- Add docs/RELEASE.md explaining the workflow & process
- `create_installer.sh`: Update to work with the release workflow
- `create_installer.sh` & `tag_release.sh`: Fix the ANSI escape codes for macOS
- `tag_release.sh`: Add check for python binary name
- `tag_release.sh`: Print `git remote -v` output
- `tag_release.sh`: Fix error when deleting nonexistant tags
Also update RELEASE.md accordingly, and make the release.yml workflow match on `v*` tags.
The timeouts are at least 3x the expected time to complete the jobs.

This is particularly relevant for the `pytest` job. Occasionally, it hangs while running tests that do network things, and the job only times out after 6 hours.
If in CI, print a message saying so.

If not, prompt user to confirm that they are in the correct working directory.
@psychedelicious
Copy link
Collaborator Author

Superseded by #5839

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD documentation Improvements or additions to documentation installer PRs that change the installer Root
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants