Implement properties fetch for GitLab #4446
Conversation
JAORMX
force-pushed
the
gitlab-fetch-properties
branch
from
47f51a4
to
05bbae7
Compare
Minder Vulnerability Report ✅Minder analyzed this PR and found it does not add any new vulnerable dependencies.
|
JAORMX
force-pushed
the
gitlab-fetch-properties
branch
from
05bbae7
to
47167e8
Compare
| return nil, fmt.Errorf("failed to create properties: %w", err) | ||
| } | ||
|
|
||
| return outProps.Merge(getByProps), nil |
There was a problem hiding this comment.
should we merge getByProps as-is or just cherry-pick the known values?
There was a problem hiding this comment.
I don't have a strong opinion, as long as outProps takes precedence.
There was a problem hiding this comment.
what maybe is not apparent: merge replaces any properties in outProps with same-named properties in getByProps so the caller could essentially set the outProps...
There was a problem hiding this comment.
Ah! So I meant to do the opposite then.
There was a problem hiding this comment.
I acked the code, but I took a note to revisit how we actually use Merge and see if we either 1) change Merge to not replace keys that exist or 2) add something like MergeIfNotExists (or call it Extend like Python)
This does a very basic implementation of fetching the base propertiesf or a repo in gitlab. Closes: #4330 Signed-off-by: Juan Antonio Osorio <[email protected]>
JAORMX
force-pushed
the
gitlab-fetch-properties
branch
from
47167e8
to
88f51fb
Compare
| return nil, fmt.Errorf("upstream ID not found or invalid: %w", err) | ||
| } | ||
|
|
||
| projectURLPath, err := url.JoinPath("projects", url.PathEscape(uid)) |
There was a problem hiding this comment.
I guess later we'll split this into a helper when we have more getProperties calls
There was a problem hiding this comment.
yep, all of this will get refactored in due time.
Summary
This does a very basic implementation of fetching the base propertiesf
or a repo in gitlab.
Closes: #4330
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: