Skip to content

Updated Dependabot to look at GHA, Pants Python, and NPM deps#22771

Merged
sureshjoshi merged 7 commits intopantsbuild:mainfrom
sureshjoshi:dependabot-3rd-party
Oct 24, 2025
Merged

Updated Dependabot to look at GHA, Pants Python, and NPM deps#22771
sureshjoshi merged 7 commits intopantsbuild:mainfrom
sureshjoshi:dependabot-3rd-party

Conversation

@sureshjoshi
Copy link
Member

@sureshjoshi sureshjoshi commented Oct 21, 2025

Pretty big change, some of which I'm still working out - but it's better than it was before. Highlights:

  • Groups of PRs instead of distinct ones (this could mean merging is less trivial until we catch up on the backlog - but WAY less noise)
  • Trying to carve out security updates from version updates, but I think until we stabilize, it will be harder to capture those right now (I might also need to mess with the dependency types and patterns someday)

Other comments in code.

Sample PR screenshots from my repo:

Screenshot 2025-10-21 at 22 38 42 Screenshot 2025-10-21 at 22 39 13 Screenshot 2025-10-21 at 22 39 50

@sureshjoshi sureshjoshi added category:internal CI, fixes for not-yet-released features, etc. release-notes:not-required [CI] PR doesn't require mention in release notes labels Oct 21, 2025
@sureshjoshi
Copy link
Member Author

Annoyingly, Dependabot will run on changes - but not from a PR, from what I can tell. Looking into https://github.com/dependabot/cli

@sureshjoshi
Copy link
Member Author

Well, I don't like that CLI one bit

@sureshjoshi sureshjoshi changed the title Attempting to get Dependabot looking at the Pants Python deps Updated Dependabot to look at GHA, Pants Python, and NPM deps Oct 22, 2025
interval: "weekly"
day: "wednesday"
time: "03:00"
timezone: "US/Pacific"
Copy link
Member Author

Choose a reason for hiding this comment

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

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:
Copy link
Member Author

Choose a reason for hiding this comment

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

Only have default reviewers for Rust, as I don't want to hammer everyone with updates

allow:
- dependency-type: "all"
groups:
rust-security-updates:
Copy link
Member Author

Choose a reason for hiding this comment

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

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

@sureshjoshi
Copy link
Member Author

Bump @benjyw @cognifloyd @tdyas @cburroughs
This PR is in a position to be reviewed and bike shedded

@cognifloyd
Copy link
Member

cognifloyd commented Oct 22, 2025

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 (workflow_run) or right after it creates a PR (pull_request_target). The workflow would generate the lockfile with pants, commit it with the lockfile diff in the commit message, and add a comment on the PR with the lockfile diff.

I created a workflow_dispatch workflow in the StackStorm project that does some of this. That one only runs when requested and either creates a new PR or pushes the commit to an existing PR. It has a lot of StackStorm-specific bits, so I didn't submit it to pantsbuild/actions.
https://github.com/StackStorm/st2/blob/master/.github/workflows/lockfiles.yaml

I added that workflow in this PR.
StackStorm/st2#6356

@sureshjoshi
Copy link
Member Author

Will dependabot update the js and cargo lockfiles? Glancing at the docs, I think it does.

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.

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 (workflow_run) or right after it creates a PR (pull_request_target). The workflow would generate the lockfile with pants, commit it with the lockfile diff in the commit message, and add a comment on the PR with the lockfile diff.

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 cargo audit a lot of that just flops.

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.

@sureshjoshi
Copy link
Member Author

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.

Copy link
Member

@cognifloyd cognifloyd left a comment

Choose a reason for hiding this comment

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

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.

@sureshjoshi
Copy link
Member Author

sureshjoshi commented Oct 22, 2025

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: fastapi==0.78.0 while it's currently on fastapi==0.119.1 - so that hasn't been updated since May 2022... I'll be honest, other than occasional pex updates - I forget that file even exists.

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.

Copy link
Contributor

@cburroughs cburroughs left a comment

Choose a reason for hiding this comment

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

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
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do? Is it intended to just be things with an CVE? (The screenshot looked like a wider net)

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Spirit being, security updates are more urgent, generally fewer breaking changes, and overall quicker to land

@sureshjoshi
Copy link
Member Author

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?

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.

@sureshjoshi
Copy link
Member Author

So, not sure where this is left off. Should we not have a Dependabot that signals more dated deps? Or what's the issue?

Copy link
Contributor

@cburroughs cburroughs left a comment

Choose a reason for hiding this comment

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

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.

@sureshjoshi
Copy link
Member Author

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.

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.

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.

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.

@sureshjoshi sureshjoshi merged commit 6a62317 into pantsbuild:main Oct 24, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:internal CI, fixes for not-yet-released features, etc. release-notes:not-required [CI] PR doesn't require mention in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants