Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions providers/git/src/airflow/providers/git/hooks/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def get_ui_field_behaviour(cls) -> dict[str, Any]:
return {
"hidden_fields": ["schema"],
"relabeling": {
"login": "Username",
"login": "Username or Access Token name",
"host": "Repository URL",
"password": "Access Token (optional)",
},
Expand All @@ -68,6 +68,7 @@ def __init__(
super().__init__()
connection = self.get_connection(git_conn_id)
self.repo_url = repo_url or connection.host
self.user_name = connection.login or "user"
Copy link
Contributor

Choose a reason for hiding this comment

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

@codenamelxl it definitely seems wrong to have a default of "user" for username

why does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for the later logic which always enforces a "user_name".

self.auth_token = connection.password
self.private_key = connection.extra_dejson.get("private_key")
self.key_file = connection.extra_dejson.get("key_file")
Expand All @@ -89,7 +90,7 @@ def _process_git_auth_url(self):
if not isinstance(self.repo_url, str):
return
if self.auth_token and self.repo_url.startswith("https://"):
self.repo_url = self.repo_url.replace("https://", f"https://{self.auth_token}@")
self.repo_url = self.repo_url.replace("https://", f"https://{self.user_name}:{self.auth_token}@")
Copy link
Contributor

Choose a reason for hiding this comment

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

what if user is not specified, and you don't want it to be specified?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original one does not work for gitlab.
It requires a user_name even for auth_token case.
For github, either no user_name or bogus user_name works.

So I was thinking specifying it like this will achieve the most compatibility. Please refer to: #51256 (comment)

elif not self.repo_url.startswith("git@") or not self.repo_url.startswith("https://"):
self.repo_url = os.path.expanduser(self.repo_url)

Expand Down
4 changes: 2 additions & 2 deletions providers/git/tests/unit/git/hooks/test_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,11 @@ def setup_class(cls) -> None:
"conn_id, hook_kwargs, expected_repo_url",
[
(CONN_DEFAULT, {}, AIRFLOW_GIT),
(CONN_HTTPS, {}, f"https://{ACCESS_TOKEN}@github.com/apache/airflow.git"),
(CONN_HTTPS, {}, f"https://user:{ACCESS_TOKEN}@github.com/apache/airflow.git"),
(
CONN_HTTPS,
{"repo_url": "https://github.com/apache/zzzairflow"},
f"https://{ACCESS_TOKEN}@github.com/apache/zzzairflow",
f"https://user:{ACCESS_TOKEN}@github.com/apache/zzzairflow",
),
(CONN_ONLY_PATH, {}, "path/to/repo"),
],
Expand Down