Fix get_packages by osp-project to cover non-url strings #172
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
Currently, the urlparse lib is used to find the short repo name from the osp-patches string in distroinfo. This does not work for non-url based strings as `git@gitlab.com:eng/test/repo_name.git`. This patch is moving to use regex instead of urlparse asuming that the repo name is the string after the last `/` and removing the `.git` if exists. This should cover all kind of strings that may be used in osp-patches field. Note this is changing slightly the behavior for repos in namespaces. i.e. before this patch if osp-patches is `ssh://server.com/foo/bar`, osp-project would be `foo/bar`. After this patch, it is `bar`. This behavior matches the current usage of this parameter which is used to matcht zuul.project.short_name which points always to the last part, `bar` in the previous case. This has not been affecting in the past because the osp-patches used were not namespaced.
we don't do that here 😛 /edit/ ach, sorry, I confused with /lgtm command from operators repos, nevermind 😅 |
sdatko
reviewed
Feb 3, 2025
znoyder/browser.py
Outdated
| @@ -72,7 +74,10 @@ def get_packages(**kwargs): | |||
| if kwargs.get('upstream') in str(package.get('upstream'))] | |||
|
|
|||
| for package in packages: | |||
| package['osp-project'] = urlparse(package['osp-patches']).path[1:] | |||
| if package['osp-patches']: | |||
| repo_name = re.search(r'^(.*)\/(.*)$', package['osp-patches']).group(2) | |||
There was a problem hiding this comment.
pep8: commands[0]> flake8
./znoyder/browser.py:78:80: E501 line too long (83 > 79 characters)
There was a problem hiding this comment.
why it is under conditional? This may change the original behavior – the osp-project shall be always defined, based on osp-patches, so if we have no osp-patches we probably have some issues in ospinfo...
There was a problem hiding this comment.
Fixed directly here to move forward with blocked work
Fixed the line to long linter error and replaced the regex replace with non-regex solution
|
Thanks! |
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.
Currently, the urlparse lib is used to find the short repo name from the osp-patches string in distroinfo. This does not work for non-url based strings as
git@gitlab.com:eng/test/repo_name.git.This patch is moving to use regex instead of urlparse asuming that the repo name is the string after the last
/and removing the.gitif exists. This should cover all kind of strings that may be used in osp-patches field.Note this is changing slightly the behavior for repos in namespaces. i.e. before this patch if osp-patches is
ssh://server.com/foo/bar, osp-project would befoo/bar. After this patch, it isbar. This behavior matches the current usage of this parameter which is used to matcht zuul.project.short_name which points always to the last part,barin the previous case. This has not been affecting in the past because the osp-patches used were not namespaced.