Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ssh to http conversion for azure devops repos #185

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix ssh to http conversion for azure devops repos #185

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
04fa8a9
Fix ssh to http conversion for azure devops repos
mads-oestergaard Jan 29, 2024
f640eb6
Move ssh filter into regex and use list.insert in path
mads-oestergaard Feb 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions clearml_agent/helper/repo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ def remote_branch_name(branch):
(?:(?P<user>{regular}*?)@)?
(?P<host>{regular}*?)
:
(?:v3/)? # present in azure ssh urls
Copy link
Member

Choose a reason for hiding this comment

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

@mads-oestergaard how is this actually used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It keeps :v3/ out of the path group.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's used down in line 254

(?P<path>{regular}.*)?
$
""".format(
Expand Down Expand Up @@ -253,6 +254,16 @@ def get_username(user_, password=None):
match = cls.SSH_URL_GIT_SYNTAX.match(url)
if match:
user, host, path = match.groups()

# handle special azure cases
if "ssh" and "azure" in host:
Copy link
Member

Choose a reason for hiding this comment

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

This seems pretty specific and I'm not sure we can count on it not appearing in other URLs - I think we need a better way to detect that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sure. I'm just not aware of other URLs that has this pattern.

Generally I guess that you wouldn't expect [.?]ssh[.?] to show up in the host path in any case and then we only need to insert the /_git/ thing for azure URLS?

Copy link
Contributor Author

@mads-oestergaard mads-oestergaard Jan 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We could also add a match in the SSH_URL_GIT_SYNTAX regex that filters out ssh. from the path group and then only handle the special _git component here. WDYT @jkhenning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make it more general, I suppose one could filter for v followed by a single digit, instead of v3 directly, to mitigate v4 and v5 etc. It does feel over-engineered though.

Copy link
Member

Choose a reason for hiding this comment

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

@mads-oestergaard I think we should just go with an exact string representing the Azure URL (this can be in the default configuration so people might override it if things change)

host = host.replace("ssh.", "")

# azure http url is different than ssh url
# the format is https://dev.azure.com/{organization}/{project}/_git/{repo}
path_components = path.split("/")
path = "/".join(path_components[:-1]) + "/_git/" + path_components[-1]

return (
furl()
.set(scheme="https", username=get_username(user), host=host, path=path)
Expand Down
1 change: 1 addition & 0 deletions tests/package/ssh_conversion.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
("ftp://example.com/a/b/", None),
("github.com:foo/bar.git", "https://github.com/foo/bar.git"),
("[email protected]:foo/bar.git", "https://github.com/foo/bar.git"),
("[email protected]:v3/org/project/repo", "https://dev.azure.com/org/project/_git/repo"),
("bitbucket.org:foo/bar.git", "https://bitbucket.org/foo/bar.git"),
("[email protected]:foo/bar.git", "https://bitbucket.org/foo/bar.git"),
("ssh://bitbucket.org/foo/bar.git", "https://bitbucket.org/foo/bar.git"),
Expand Down