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.

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

feat(go/adbc/driver/snowflake): add '[ADBC]' to snowflake application name #1525

Merged
merged 6 commits into from
Feb 7, 2024

Conversation

zeroshade
Copy link
Member

To help Snowflake track adoption and usage of the ADBC driver, we can explicitly add a prefix to any client application name to indicate the ADBC driver is the source of the requests.

Copy link

github-actions bot commented Feb 7, 2024

⚠️ Please follow the Conventional Commits format in CONTRIBUTING.md for PR titles.

@github-actions github-actions bot added this to the ADBC Libraries 0.10.0 milestone Feb 7, 2024
@lidavidm lidavidm changed the title chore(snowflake): add '[ADBC]' to snowflake application name feat(go/adbc/driver/snowflake): add '[ADBC]' to snowflake application name Feb 7, 2024
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Oh, you're trying to override the user choice...do we want that? It seems to make more sense to just pass along the user's application name if set, otherwise set a basic one indicating the driver.

@zeroshade
Copy link
Member Author

My intent was to prefix to the users choice so we can still track it, but I'm fine with just having it not override at all if we think that's better.

@lidavidm
Copy link
Member

lidavidm commented Feb 7, 2024

IMO if gosnowflake doesn't prefix we shouldn't either (does it?); and in Go and Python we should set defaults if they're not set by the user (so Snowflake can track the Go/Python drivers otherwise)

@lidavidm
Copy link
Member

lidavidm commented Feb 7, 2024

If only there were separate user-agent and application fields

@zeroshade
Copy link
Member Author

gosnowflake has an internal application name that is not externally changeable so regardless of the User's application name, snowflake can track it's coming from the Go driver. So they don't need to prefix

@lidavidm
Copy link
Member

lidavidm commented Feb 7, 2024

Ah...and we can't get access to that? 😅 It would be nice if we could expose this sort of thing separately

go/adbc/driver/snowflake/snowflake_database.go Outdated Show resolved Hide resolved
@zeroshade
Copy link
Member Author

Ah...and we can't get access to that?

Yeap. We can't get access to that unfortunately. They said the preferred method would be to pass it as the application name or prefix the user's choice.

@lidavidm
Copy link
Member

lidavidm commented Feb 7, 2024

Ah...and we can't get access to that?

Yeap. We can't get access to that unfortunately. They said the preferred method would be to pass it as the application name or prefix the user's choice.

Ah, sorry...if they are OK with prefixing the user's choice then I'm ok with that too.

zeroshade and others added 2 commits February 7, 2024 12:50
@zeroshade zeroshade merged commit 21fba98 into apache:main Feb 7, 2024
54 checks passed
@zeroshade zeroshade deleted the app-name-snowflake branch February 7, 2024 18:30
lidavidm pushed a commit that referenced this pull request Feb 12, 2024
A follow-up to #1525 to add an
R-specific prefix for the Snowflake driver. We could allow this to be
customized, too, but I would prefer to that feature to be requested
before pursuing it.

Closes #1526.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants