Skip to content

Conversation

@codenamelxl
Copy link
Contributor

@codenamelxl codenamelxl commented May 31, 2025

closes: #50941
add user_name into http auth link


@codenamelxl
Copy link
Contributor Author

Failed test seems not related to the change?

@potiuk
Copy link
Member

potiuk commented Jun 5, 2025

Failed test seems not related to the change?

Rebase to see .

@potiuk potiuk merged commit 5217c61 into apache:main Jun 14, 2025
97 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Jun 14, 2025

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

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".

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The git password/token for GitDagBundle when connecting to Gitlab currently needs to include the token name

3 participants