stop processing if updated_deps is empty #8193
Conversation
| if updated_deps.empty? | ||
| return Dependabot.logger.info( | ||
| "No update possible for #{dependency.name} #{dependency.version}" | ||
| ) | ||
| end |
There was a problem hiding this comment.
Under what situation can this happen? A test would help
There was a problem hiding this comment.
I don't understand either why this is needed. If there's no update possible, we should be returning an :update_not_possible result from the UpdateChecker (a few lines above this).
If that's not happening, that sounds like a bug in the update checking process.
There was a problem hiding this comment.
We found an instance internally where the NuGet updater errored out and the result was an empty update set. While we fixed the specific issue we had, there still remains the possibility that another updater could also error out and result in an empty set, and that's why we ultimately added this check as well, to prevent the pr creator from crashing. If there is a better place for this check we can certainly move it.
There was a problem hiding this comment.
I see.
I think it makes sense to avoid crashing randomly while trying to create a PR. However, I don't think this is an expected error condition, but a situation that's pointing at a Dependabot bug somewhere.
So, instead of just logging a message, we should be raising an error if this happens. Maybe something like:
raise "Dependabot found some dependency requirements to unlock, yet it failed to update any dependencies"
ryanbrandenburg
force-pushed
the
dev/brettfo/empty-update
branch
2 times, most recently
from
770f4d7
to
cd0b8ad
Compare
github-actions
bot
added
L: go:modules
|
I have adjusted the change per your suggestion, added a test for this scenario, and updated the branch. The PR should be ready for re-review. |
Generating the PR title errors when updated_deps is empty. Related work items: #2087651
JamieMagee
force-pushed
the
dev/brettfo/empty-update
branch
from
80af328
to
918a33e
Compare
updater/spec/dependabot/updater/operations/update_all_versions_spec.rb
Outdated
Show resolved
Hide resolved
updater/spec/dependabot/updater/operations/update_all_versions_spec.rb
Outdated
Show resolved
Hide resolved
updater/spec/dependabot/updater/operations/empty_return_update_checker.rb
Outdated
Show resolved
Hide resolved
As part of the work inside Azure DevOps to improve dependabot (see #8179) there were other updates that needed to be split out into their own PR, specifically not continuing with an update if the updated set was empty.