Skip to content

Create tag with constraints files for new releases #6495

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.

Sign up for GitHub

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

Jump to bottom
Merged
sfc-gh-mnowotka merged 4 commits into from
Apr 18, 2023
Merged

Create tag with constraints files for new releases #6495

sfc-gh-mnowotka merged 4 commits into from
Apr 18, 2023

Conversation

@sfc-gh-kbregula
Copy link
Contributor

@sfc-gh-kbregula sfc-gh-kbregula commented Apr 15, 2023 •
edited
Loading

📚 Context

Hi. I make a few changes so that the constraints files are automatically generated for each new release.

This PR contains the following changes:

  • Remove .github/actions/tag_and_repo action
    I deleted this action because it made creating new environment variables used by all build steps too complicated. Github Action has an env field that we can use in this case. A separate issue is whether we should create these variables at all because it looks like a duplicate of the default environment variables (GITHUB_REF_NAME, GITHUB_REPOSITORY) created by Github Action. I didn't remove these variables to limit the scope.

  • Make "Build Release" workflow run in private forks
    To make it easier to test the "Release build" workflow, I made a few changes to make this workflow work on private forks. Especially, if the GitHub token is available, it is passed to HTTP requests and the correct permissions are set for the Github Action token.

    In private forks, only the step "Upload to PyPI" is not running, because PyPi is a public package repository, so we don't want to put private artifacts in there.

  • Create a tag with constraints files for new releases.
    The "Release build" workflow now runs tests against all Python versions to generate the constraints. This file is then placed on the constraints-release branch.If the constraints files have changed since the last release, then a new commit is created. And then a tag is created based on the latest commit.

    The constraints files are stored in a separate branch, because theoretically on the develop branch, we can support newer dependencies than on release. We will also have a clearer history because now all user-facing constraints files are on the single branch - constraints-release.

🌐 References

Does this depend on other work, documents, or tickets?

  • Issue: Closes #XXXX

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-kbregula sfc-gh-kbregula force-pushed the create-tag-for-release branch 7 times, most recently from 6c2bbc5 to 06f6ef7 Compare April 17, 2023 09:50
@sfc-gh-kbregula sfc-gh-kbregula changed the title Create tag with constraints files for release Create tag with constraints files for new releases Apr 17, 2023
@sfc-gh-kbregula sfc-gh-kbregula force-pushed the create-tag-for-release branch 2 times, most recently from e1ece0a to 1881cdd Compare April 17, 2023 10:22
@sfc-gh-tszerszen sfc-gh-tszerszen self-requested a review April 17, 2023 10:51
sfc-gh-tszerszen
sfc-gh-tszerszen approved these changes Apr 17, 2023
View reviewed changes
Copy link
Contributor

@sfc-gh-tszerszen sfc-gh-tszerszen left a comment

Choose a reason for hiding this comment

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

LGTM! 🥇

@sfc-gh-kbregula sfc-gh-kbregula marked this pull request as ready for review April 17, 2023 13:03
@sfc-gh-kbregula sfc-gh-kbregula force-pushed the create-tag-for-release branch 3 times, most recently from dc04dee to a3eeedf Compare April 17, 2023 15:12
@vdonato
Copy link
Collaborator

vdonato commented Apr 18, 2023

A separate issue is whether we should create these variables at all because it looks like a duplicate of the default environment variables (GITHUB_REF_NAME, GITHUB_REPOSITORY) created by Github Action. I didn't remove these variables to limit the scope.

I'd be in favor of removing the variables entirely if they are indeed just duplicates of default env vars, but yeah it should probably be done in a separate PR.

vdonato
vdonato approved these changes Apr 18, 2023
View reviewed changes
Copy link
Collaborator

@vdonato vdonato left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This LGTM from a code standpoint, but I think we'll want to run this by ProdSec since we're passing GitHub tokens around + adding secrets: inherit to a bunch of places. (CC @sfc-gh-hpathak)

I think this should be fine since

  • tokens are created for individual workflow runs so should have pretty restricted permissions when the token corresponds to a fork (I'm also assuming they should be short-lived given their intended use-case)
  • we require approval from a maintainer to run CI for a PR from a contributor

but I'd prefer to err on the side of caution here. If this turns out this requires some security process or has nontrivial security implications, then I'd suggest splitting 6056a79786bac032ce1d80835b9cc19ef4f93ed9 out into a separate PR so that the rest of it can be merged more quickly.

@sfc-gh-kbregula sfc-gh-kbregula force-pushed the create-tag-for-release branch from a3eeedf to 72c0100 Compare April 18, 2023 06:08
@sfc-gh-kbregula sfc-gh-kbregula mentioned this pull request Apr 18, 2023
Closed
9 tasks
@sfc-gh-kbregula
Copy link
Contributor Author

sfc-gh-kbregula commented Apr 18, 2023

I have moved changes that require prod sec reviews to a separate PR. Once this change is merged, I will do a rebase and ask for reviews.
#6506

@sfc-gh-kbregula sfc-gh-kbregula mentioned this pull request Apr 18, 2023
Merged
@sfc-gh-kbregula sfc-gh-kbregula added the security-assessment-completed Security assessment has been completed for PR label Apr 18, 2023
@sfc-gh-mnowotka sfc-gh-mnowotka merged commit dbcde30 into develop Apr 18, 2023
tconkling added a commit to tconkling/streamlit that referenced this pull request Apr 18, 2023
@tconkling
Merge branch 'develop' into tim/FrontendAppLibSplit
d7a9617 
* develop:
  Create tag with constraints files for new releases (streamlit#6495)
  Autoformat yaml in .github dir and enforce formatting (streamlit#6497)
  Update metadata description (streamlit#6503)
  Update CODEOWNERS (streamlit#6496)
@sfc-gh-kmcgrady sfc-gh-kmcgrady deleted the create-tag-for-release branch October 5, 2023 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vdonato vdonato vdonato approved these changes

@sfc-gh-mnowotka sfc-gh-mnowotka Awaiting requested review from sfc-gh-mnowotka

@mayagbarnes mayagbarnes Awaiting requested review from mayagbarnes

@kmcgrady kmcgrady Awaiting requested review from kmcgrady

+1 more reviewer

@sfc-gh-tszerszen sfc-gh-tszerszen sfc-gh-tszerszen approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sfc-gh-kbregula @vdonato @sfc-gh-tszerszen @sfc-gh-mnowotka