jedcunningham commented on code in PR #45126:
URL: https://github.com/apache/airflow/pull/45126#discussion_r1895183480
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -126,3 +126,29 @@ def refresh(self) -> None:
self.bare_repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
self.repo.remotes.origin.pull()
+
+ def _convert_git_ssh_url_to_https(self) -> str:
+ if self.repo_url.startswith("git@"):
+ parts = self.repo_url.split(":")
+ domain = parts[0].replace("git@", "https://";)
+ repo_path = parts[1].replace(".git", "")
+ return f"{domain}/{repo_path}"
+ raise ValueError(f"Invalid git SSH URL: {self.repo_url}")
+
+ def view_url(self, version: str | None = None) -> str:
+ if not version:
+ raise AirflowException("Version is required to view the
repository")
Review Comment:
Typing allowing none versions, but having it always raise, feels a bit
awkward. Perhaps we just return none in this case?
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -126,3 +126,29 @@ def refresh(self) -> None:
self.bare_repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
self.repo.remotes.origin.pull()
+
+ def _convert_git_ssh_url_to_https(self) -> str:
+ if self.repo_url.startswith("git@"):
+ parts = self.repo_url.split(":")
+ domain = parts[0].replace("git@", "https://";)
+ repo_path = parts[1].replace(".git", "")
+ return f"{domain}/{repo_path}"
+ raise ValueError(f"Invalid git SSH URL: {self.repo_url}")
+
+ def view_url(self, version: str | None = None) -> str:
+ if not version:
+ raise AirflowException("Version is required to view the
repository")
+ if not self._has_version(self.repo, version):
+ raise AirflowException(f"Version {version} not found in the
repository")
+ url = self.repo_url
+ if url.startswith("git@"):
+ url = self._convert_git_ssh_url_to_https()
+ if url.endswith(".git"):
+ url = url[:-4]
+ if "github" in url:
Review Comment:
```suggestion
if url.startswith("https://github.com";):
```
I wonder if this might be a bit more resilient?
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -126,3 +126,29 @@ def refresh(self) -> None:
self.bare_repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
self.repo.remotes.origin.pull()
+
+ def _convert_git_ssh_url_to_https(self) -> str:
+ if self.repo_url.startswith("git@"):
+ parts = self.repo_url.split(":")
+ domain = parts[0].replace("git@", "https://";)
+ repo_path = parts[1].replace(".git", "")
+ return f"{domain}/{repo_path}"
+ raise ValueError(f"Invalid git SSH URL: {self.repo_url}")
+
+ def view_url(self, version: str | None = None) -> str:
+ if not version:
+ raise AirflowException("Version is required to view the
repository")
+ if not self._has_version(self.repo, version):
+ raise AirflowException(f"Version {version} not found in the
repository")
Review Comment:
Not sure we should even check - we are just building a url, let the other
side display a 404.
##########
airflow/dag_processing/bundles/git.py:
##########
@@ -126,3 +126,29 @@ def refresh(self) -> None:
self.bare_repo.remotes.origin.fetch("+refs/heads/*:refs/heads/*")
self.repo.remotes.origin.pull()
+
+ def _convert_git_ssh_url_to_https(self) -> str:
+ if self.repo_url.startswith("git@"):
+ parts = self.repo_url.split(":")
+ domain = parts[0].replace("git@", "https://";)
+ repo_path = parts[1].replace(".git", "")
+ return f"{domain}/{repo_path}"
+ raise ValueError(f"Invalid git SSH URL: {self.repo_url}")
Review Comment:
```suggestion
if not self.repo_url.startswith("git@"):
raise ValueError(f"Invalid git SSH URL: {self.repo_url}")
parts = self.repo_url.split(":")
domain = parts[0].replace("git@", "https://";)
repo_path = parts[1].replace(".git", "")
return f"{domain}/{repo_path}"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
