Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix: update release workflow to follow pypa and pypi guidelines #73
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
Fix: update release workflow to follow pypa and pypi guidelines #73
Changes from 11 commits
81ed956
f7ae35d
878094c
cf61545
aa4c259
ddc5597
48f3cc2
375b29e
471236b
00884d7
ad839c5
62d7ef4
022b6c7
4d535ad
6dce9fe
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This is a new one for me, so I'm curious. Could you please share how it works?
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.
Oh! @agriyakhetarpal environments allow you to setup a trusted connection with pypi. This is the build environment, so we don't need to worry too much about security here in terms of PYPI. so anyone can run the build step in theory. but by having a publish environment, we can control who can trigger that step of the build. (if that is what you're asking) .
here is a bit more about this
https://docs.pypi.org/trusted-publishers/using-a-publisher/
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.
Oh, I'm aware of trusted publishing and environments, thanks! My question was more around the need for this
build
environment, as it seems to be redundant. Most release workflows can get by with just onepypi
(orpublish
, or any other similar name) environment, omitting the need for an environment for the build step. Does including two environments, one for building and one for publishing, have security provisions? Moreover, I imagine that the users would also have questions around why they need (and we recommend) to set up two environments in their repository's settings, when it is only the publishing one that connects to PyPI's OIDC publishing functionalities.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.
Oh gosh my apologies. i suspected I was telling you something you already knew. 🙈
This is a great point. I don't know the answer to your question!! But i suspect this is ME introducing complexity.
i just checked the tutorials and there is a build step but not an additional environment. Shall we remove this environment as you are correct it's not required. we need the job but not an environment AFAIK .
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.
No worries at all! 😄
Thank you for dropping it!
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.
Yes, please. I'll recommend that all GitHub Actions—whether by GitHub as an actor or external ones available on the marketplace—should be pinned to their commit hashes, at least for a workflow concerning a release. There is a case to be made about how it makes things a bit more challenging for new contributors/packagers, but I think supply-chain security shouldn't be at the end of a compromise.
There are tools such as
gha-update
which can replace them with pins, that we can recommend elsewhere (not in this PR, though, of course). Once this is done, I think it's just a matter of letting Dependabot (if configured) take over, as it can handle both the upgrade and the inline version comment in its automated PRs.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.
great. So, looking at the action here
Ofek suggests that too - we'd use this - this hash is almost the most recent but the most recent is just a readme update.
name: Install Hatch
uses: pypa/hatch@a3c83ab
But now we'd have to update that hash manually? How often should that update happen?
We should do that as part of this repo's maintenance, but also, we'll want to tell users to update this periodically, right?
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.
Updating it weekly for our maintenance could be a good cadence; what do you think? It would depend on the amount of activity we have on this template.
If we were to ask our users to enable Dependabot in the GitHub settings and inherit the same settings, they should also get the same updates. However, I'm not sure whether it's too much to ask for beginners (😕).
As long as it is pinned for a start, we can proceed with a small guide on how to keep this workflow maintained, where we can describe this more (or link to an external guide if that's alright and vetted by us?).
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.
@agriyakhetarpal this is awesome. We, I think, have Depdendabot set up in the org, I think, but I didn't configure it. This is a basic question but would it help update things like this version? IE could we use it in this repo?
My gut tells me we won't have the bandwidth to perform weekly updates here unless someone wants to take that one! But perhaps monthly or quarterly would work. I am not sure what is best, however and would love suggestions.
also if you were interested in helping us maintain here, that would be awesome (only if you have bandwidth!)
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.
Oh, I notice that here are a few moving parts, here. As this file is not a workflow of its own but a template that will be generated and moved into
.github/workflows/
, I am not sure if Dependabot will be able to operate on the template file as well, in addition to the workflow file. It does have adirectory:
setting, which I think we should try out: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/dependabot-options-reference#directories-or-directory--That means that we have two possible scenarios:
The first one is surely not ideal, so we should explore if Dependabot will choose to be the star here or not 😉
I agree with you. Monthly updates would be nicer – and users can always bump it to daily, or weekly, and so on, based on their bandwidth.
I'd love to! I've also helped out with the development and maintenance of especially other
copier
templates, such as https://github.com/pybamm-team/pybamm-cookie – where my experience should be transferable enough for me to help out. I do have limited bandwidth, however and can't say I can work full-time on this, but I would be open to lend a hand with more reviews and occasional updates anytime :DThere 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.
Ok wonderful!! @agriyakhetarpal i'm going to create an issue for thiw topic specifically as it's beyon the scope of this pr and merging this would be nice!! BUT it's an important topic. So let's move your comment above to an issue.
Also i'd love to add you to our packaging council team here at pyOpenSci which will give ou more superpowers in our repos!! I"ll do that now. So happy to have you here and also happy that @Midnighter is able to help me undstand some of the technical points and updates that you have so carefully pointed out and fixed!! thank you both!