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

support custom socket factory #1420

Merged
merged 2 commits into from
Aug 28, 2023
Merged

support custom socket factory #1420

merged 2 commits into from
Aug 28, 2023

Conversation

zhicwu
Copy link
Contributor

@zhicwu zhicwu commented Aug 14, 2023

Summary

Support custom socket factory as requested in #1391.

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@zhicwu zhicwu requested a review from mzitnik August 14, 2023 09:58
@mzitnik
Copy link
Contributor

mzitnik commented Aug 14, 2023

@zhicwu can you provide a complete code sample so we could add it to documentation

@zhicwu
Copy link
Contributor Author

zhicwu commented Aug 15, 2023

@zhicwu can you provide a complete code sample so we could add it to documentation

There's test(example) as well as default implementation. Do we still need complete code sample? I didn't see that happening in the server repo :)

Anyway, this was requested by @mayankvadariya, so let's wait for his comments before merging the PR.

@zhicwu zhicwu marked this pull request as draft August 15, 2023 00:32
@zhicwu zhicwu marked this pull request as ready for review August 28, 2023 10:32
@sonarcloud
Copy link

sonarcloud bot commented Aug 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 12 Code Smells

80.4% 80.4% Coverage
1.0% 1.0% Duplication

@zhicwu
Copy link
Contributor Author

zhicwu commented Aug 28, 2023

@mzitnik, can we merge this PR to close the issue?

@mzitnik mzitnik merged commit 0c80cdd into main Aug 28, 2023
60 checks passed
@zhicwu zhicwu deleted the custom-socket-factory branch August 28, 2023 10:57
@mayankvadariya
Copy link

Thank you @zhicwu for this effort. It slipped as I had been busy lately.

@zhicwu
Copy link
Contributor Author

zhicwu commented Aug 29, 2023

Thank you @zhicwu for this effort. It slipped as I had been busy lately.

No problem. Let us know if it fits your need.

@mayankvadariya
Copy link

Yes, I tested the changes locally. 🙌 Please let me know when can we release these changes.

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