Fetching GitLab repo contents correctly uses the ref argument #7351
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maciej-gol
force-pushed
the
mgol/gitlab_repo_tree_correctly_lists_files_for_ref
branch
from
a07f879
to
a35fd73
Compare
|
Mind having a look into this? |
|
The change looks good to me, we have some internal stability work that the team is prioritizing so I'll need to hold off merging this for a bit. One thing that I would love to see is some sort of test that would catch this in the future. |
maciej-gol
force-pushed
the
mgol/gitlab_repo_tree_correctly_lists_files_for_ref
branch
from
a35fd73
to
df059fb
Compare
|
Will try to have a look into adding tests, Im not really proficient with ruby |
jurre
force-pushed
the
mgol/gitlab_repo_tree_correctly_lists_files_for_ref
branch
from
df059fb
to
4a49493
Compare
jeffwidman
force-pushed
the
mgol/gitlab_repo_tree_correctly_lists_files_for_ref
branch
from
4a49493
to
8062d1e
Compare
jeffwidman
approved these changes
Aug 3, 2023
There was a problem hiding this comment.
While a test would be nice, I'm personally okay shipping this as-is w/o any tests because the impact of a regression is relatively small:
- Won't impact our cloud service at GitHub at all
- Impact for GitLab users is primarily when their testing on a branch other than
main - It clearly hasn't been a blocker for folks so far, so it's more of a nice-to-have
- The Gitlab PR creator is very much community maintained.
- Long term, we may eventually move away from Ruby-based PR creators to instead doing something with the CLI where it takes job output and has pluggable PR creators... that's a ways off, but it seems reasonable this code may not live forever.
Anyway, all that to say, I think this is fine to ship. If a teammate comes along and feels differently, then I can try to add a test if needed.
Thanks for tracking it down.
brettfo
pushed a commit
to brettfo/dependabot-core
that referenced
this pull request
Oct 11, 2023
…ot#7351) Dependabot can't properly list GitLab repo contents on a given branch/commit. This is problematic when you are, for example, testing dependabot config and the requirements files are not present on the main repository branch, but on the changed branch only. In such case, dependabot won't see expected requirements, resulting in `Dependabot::DependencyFileNotFound`. # Context According to the GitLab API (https://docs.gitlab.com/ee/api/repositories.html#list-repository-tree), you have to use the `ref` argument to list the repository at a given commit/branch. The GitLab client blindly passes all the arguments to the API itself, including the `ref_name` argument. GitLab ignores extra GET parameters, thus it wasn't erroring out.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Dependabot can properly list GitLab repo contents on a given branch/commit.
This is problematic when you are, for example, testing dependabot config and the requirements files are not present on the main repository branch, but on the changed branch only. In such case, dependabot won't see expected requirements, resulting in
Dependabot::DependencyFileNotFound.Context
According to the GitLab API (https://docs.gitlab.com/ee/api/repositories.html#list-repository-tree), you have to use the
refargument to list the repository at a given commit/branch.The GitLab client blindly passes all the arguments to the API itself, including the
ref_nameargument. GitLab ignores extra GET parameters, thus it wasn't erroring out.