Updated Dependabot to look at GHA, Pants Python, and NPM deps#22771
Updated Dependabot to look at GHA, Pants Python, and NPM deps#22771sureshjoshi merged 7 commits intopantsbuild:mainfrom
Conversation
|
Annoyingly, Dependabot will run on changes - but not from a PR, from what I can tell. Looking into https://github.com/dependabot/cli |
|
Well, I don't like that CLI one bit |
| interval: "weekly" | ||
| day: "wednesday" | ||
| time: "03:00" | ||
| timezone: "US/Pacific" |
There was a problem hiding this comment.
Unnecessarily specific - implies there is currently a reason for precisely this, which isn't the case at the moment.
| timezone: "US/Pacific" | ||
| - "dependencies" | ||
| - "release-notes:not-required" | ||
| reviewers: |
There was a problem hiding this comment.
Only have default reviewers for Rust, as I don't want to hammer everyone with updates
| allow: | ||
| - dependency-type: "all" | ||
| groups: | ||
| rust-security-updates: |
There was a problem hiding this comment.
Hypothetically, this should create 2 PRs - one for security updates (important, but less invasive) - while the generic version updates are more informational and can be skipped or ignored.
Right now, it's just batching everything into version updates
|
Bump @benjyw @cognifloyd @tdyas @cburroughs |
|
Will dependabot update the js and cargo lockfiles? Glancing at the docs, I think it does. I don't think it can update the pex lockfiles though. Is there a way to have it run a command after updating the requirements? I don't see an option like that in the docs. Perhaps we could have a workflow that triggers right after dependabot runs ( I created a I added that workflow in this PR. |
It will certainly try, however, given the setup in the repo, we're going to have some issues (e.g. we have lockfiles without package.json files). I'll need to go in and do some work around these first, then I think the NPM one will work better (fails right now). Also, the Cargo one currently runs - this is more additive than anything else.
Yeah, it can't solve that yet, but right now, I'd at least like for it to flag PRs to us, that there are updates (e.g. 31 of them). We could also run this in regular CI, but much like Actually, I rarely actually use Dependabot's PRs - I use it as a signal that there are updates, and then I typically make the changes manually. Even with all the cargo updates, there are so many breaking changes, that just mergeing the PR rarely works. |
|
There is the security tab that has some notes, but generally having a full fat PR in front of you makes it more obvious. And just because there are so many updates right now, it's not like we'd blindly let the PR merge anyways. |
cognifloyd
left a comment
There was a problem hiding this comment.
In other projects I have pushed for disabling dependabot. Dependabot has been too painfully noisy in those projects for me to trust it, so I'm not approving this PR. But, I see that it might provide some value, so I'm not going to block it either. I'm curious to see how these grouped update PRs will play out over time, and how the PRs will affect the project. I hope those effects will be positive.
|
That's fair. I would say strictly on the basis of 31ish stale Python dependencies that Pants uses (not including subsystems), that the "current system" is a dismal failure in some regard. So, at the very least, having dependabot inform us of the situation. As mentioned, I don't think it'll do the update (nor do I necessarily want it to very often - I prefer reading changelogs and making sure there are no surprises). Great example: Re: noise: I've enabled grouping on other projects and 7-10 PRs collapse into 1. So generally, it's a rolling 1-2 PRs per package manager that stay updated after each Dependabot run (weekly). That feels super manageable, when compared against our 71 open PRs, and formerly 120+ open PRs. |
cburroughs
left a comment
There was a problem hiding this comment.
I guess I'm having trouble following the intended workflow here. We would get a weekly PR that can't be merged because it didn't update the lockfile or doesn't understand what is generated code or didn't trigger a script or something like that. Is that right?
And then what? Someone closes it? Regenerates a lockfile? Wait for a stalebot? Make tickets for specific problems?
| directory: "/3rdparty/python" | ||
| groups: | ||
| python-security-updates: | ||
| applies-to: security-updates |
There was a problem hiding this comment.
What does this do? Is it intended to just be things with an CVE? (The screenshot looked like a wider net)
There was a problem hiding this comment.
Yeah, so, what I'm trying to get setup is to split out security-only updates from general version updates. So far, it doesn't seem to be working - but that might be a result of issues being so out of date that any security update is necessarily a version update - and it gets captured there. I won't really know until this lands and we watch it for a few iterations to see how it plays out over time.
There was a problem hiding this comment.
Spirit being, security updates are more urgent, generally fewer breaking changes, and overall quicker to land
Yeah, so right now, the Rust stuff updates lockfiles and does whatever it needs to. If we want to continue using Dependabot (which, why not), then it can't fully perform our whole python upgrade cycle (and note, none of this even starts on the whole subsystem world). Cargo should remain the same, once I can fix up some of the JS/TS stuff, we miiight be able to get PRs for those files as well. However, our python stuff specifically requires also doing pants stuff. This could be accomplished with a separate action (similar to what we do today with cargo audit), but that might also be more work and more brittle (similar to cargo audit in the Pants actions). For this Dependabot workflow, what I first want to see is: "Hey, here's all your out of date Python deps! It's in the form of a PR, but since you use Pants, you'll need to do extra work". So, right now, the python Dependabot is more of a signal, than it is a fully finished "this will automatically solve all of our problems" There's probably a way to kick off extra scripts after Dependabot runs (almost certainly, since we could just point an action at the requirements.txt changing and run a command) - but that feels like solving a future problem before we've manually run through the challenges. |
|
So, not sure where this is left off. Should we not have a Dependabot that signals more dated deps? Or what's the issue? |
cburroughs
left a comment
There was a problem hiding this comment.
Thanks for the broader explanation. I think it makes sense to roll forward and see what this look like instead of staying theoretical.
To pre-register, I suspect weekly is too frequent for general updates (at least in Python land; there are probably four javascript framework migrations in that span of time) but let's see what it looks like now and at that cadence.
We are not super strict on the time based release thing, but one big regenerate all the lockfiles per release would -- I think -- be a reasonable balance of toil and update-churn.
Yeah, my original hope was that the "groups" would also be able to be at different cadences (security - fast, versions - slow) - but that doesn't seem to be the case yet, or I need a different configuration.
I think in the short term, we'll have some related subsets at a time (and maybe even removing deps), and then it'll be a lot quicker to generate and go - once we're not so far behind (as I'm sure we're in breaking change city across the board. |
Pretty big change, some of which I'm still working out - but it's better than it was before. Highlights:
Other comments in code.
Sample PR screenshots from my repo: