-
Notifications
You must be signed in to change notification settings - Fork 930
Remove ConnectionPool #5469
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
Remove ConnectionPool #5469
Conversation
|
Changeset File Check ✅
|
…s-sdk into mrschmidt/connection
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG, one nit.
@@ -22,6 +22,9 @@ import { | |||
} from '../../implementation/connection'; | |||
import { internalError } from '../../implementation/error'; | |||
|
|||
/** An override for the text-based Connection. Used in tests. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should Connection be backticked as a literal? Looks like there are other instances of the same in other files in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is part of the internal code so I have no strong opinion, other than laziness :)
Binary Size ReportAffected SDKs
Test Logs |
Size Analysis ReportAffected Products
|
3fc7c4e
to
187ef92
Compare
92f9e50
to
93001ef
Compare
This PR is meant to reduce the diff for #4973
The PR removes the old ConnectionPool, which was never used to share connections but rather just allowed tests to inject a different connection type. Since #4973 introduces multiple connection types (string, blob, stream) and since we only want to include the connection types that are actually used, this PR now directly specifies the specific connection factory at the callsite (which for now is just a text-based connection).