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

Jump to bottom

allow to provide an additional url path #512

Merged
merged 4 commits into from
Nov 9, 2023
Merged

allow to provide an additional url path #512

merged 4 commits into from
Nov 9, 2023

Conversation

tatchi
Copy link
Contributor

@tatchi tatchi commented Sep 6, 2023

Fixes #499

It turns out clickhouse-go already support this feature via an HttpUrlPath option

@tatchi tatchi requested a review from a team as a code owner September 6, 2023 23:32
@CLAassistant
Copy link

CLAassistant commented Sep 6, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

aangelisc
aangelisc reviewed Sep 7, 2023
View reviewed changes
Copy link
Contributor

@aangelisc aangelisc left a comment

Choose a reason for hiding this comment

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

This LGTM in general @tatchi, thank you for the contribution! Would you also be able to update the CHConfigEditor, driver and settings tests?

@tatchi
Copy link
Contributor Author

tatchi commented Sep 7, 2023

@aangelisc I added a few tests but I never wrote any go previously so I hope what I did makes sense. Please feel free to adjust if necessary.

asimpson
asimpson approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@asimpson asimpson left a comment

Choose a reason for hiding this comment

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

Thanks for this, it looks great and apologies again for the delay! 🏆

@asimpson asimpson enabled auto-merge (squash) November 9, 2023 19:18
@asimpson asimpson merged commit 402a6a8 into grafana:main Nov 9, 2023
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aangelisc aangelisc aangelisc left review comments

@asimpson asimpson asimpson approved these changes

@bossinc bossinc Awaiting requested review from bossinc bossinc is a code owner automatically assigned from grafana/partner-datasources

@adamyeats adamyeats Awaiting requested review from adamyeats adamyeats is a code owner automatically assigned from grafana/partner-datasources

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to add a host with proxy
4 participants
@tatchi @CLAassistant @asimpson @aangelisc