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

fix: add default value for access token in HttpClient init #448

Merged
merged 1 commit into from
Jan 7, 2025

Conversation

lukasthalerINNIO
Copy link
Contributor

Summary

0.8.12 introduced a new access_token parameter to the clickhouse_connect.driver.HttpClient class that is marked as Optional[str] but has no default value. It is properly defaulted in create_client, but if you directly create an instance of HttpClient in 0.8.12, it fails due to the missing new argument

This PR adds a None default value to the HttpClient __init__ to mirror create_client and fix the issue

Checklist

Delete items not relevant to your PR:

  • A human-readable description of the changes was provided to include in CHANGELOG: Set default value for the optional access_token parameter of the HttpClient.__init__ method

@CLAassistant
Copy link

CLAassistant commented Jan 7, 2025

CLA assistant check
All committers have signed the CLA.

@genzgd
Copy link
Collaborator

genzgd commented Jan 7, 2025

Thanks for the fix! Do you need a release with this change immediately?

@lukasthalerINNIO
Copy link
Contributor Author

A quick release would be much appreciated. We use uncapped clickhouse-connect dependencies a fair bit and all new builds are crashing due to this. I've put out a PSA to the team, but a new release would make our lives a lot easier

@genzgd genzgd merged commit 586d51f into ClickHouse:main Jan 7, 2025
33 checks passed
@lukasthalerINNIO lukasthalerINNIO deleted the patch-1 branch January 7, 2025 17:03
@genzgd
Copy link
Collaborator

genzgd commented Jan 7, 2025

I've pushed a PyPi release. Just a gentle reminder that the driver is still beta and it's recommended to use a specific, tested version for production workloads.

Yibo-Chen13 pushed a commit to timeplus-io/timeplus-connect that referenced this pull request Jan 15, 2025
Yibo-Chen13 pushed a commit to timeplus-io/timeplus-connect that referenced this pull request Jan 15, 2025
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.

3 participants