-
Notifications
You must be signed in to change notification settings - Fork 99
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
Http client conflicts #120
Comments
It seems like it was added with this PR: #49 @mohitpali is the UrlConnectionHttpClient needed for the JSON support? Have this dependency in the library conflicts with being able to use the ProfileCredentialProvider in the default configuration when this dependency is in the classpath. |
Thanks for reporting! I will investigate this and post my findings. |
I think we were specifying it explicitly to avoid conflicts with HttpClient. Are you constructing a AWS Client in your application? Related AWS SDK issue |
You're right, found where it was implicitly created, creating it explicitly seems to have fixed the problem! |
I reopened the issue; I feel like the fact that it is explicitly specified here in this dependency should not affect the rest of the codebase having glue-schema-registry as a dependency. Instead of having to specify it explicitly everywhere else, glue-schema-registry should also use the service loader approach and support several http clients. |
What is the service loader approach? |
It would be to place a provider configuration file in Dug through the aws-sdk-v2 source code and it seems it is simply not supported to have more than one http client in the classpath. Loading a provider with a provider configuration file won't work since it is done after loading any normal named modules ref java source. |
Any plans on fixing this? |
Here's how we have worked around it. We build the library with
|
Plus 1 for this. Other AWS clients resolves and HTTP client via service loader. The workaround is to to do this:
But is it really what we are looking for? Thanks for understanding! |
* Exclude `apache-client` from the `amazon-kinesis-client` to avoid conflict with an `url-connection-client` which is hard-coded to the `aws-glue-schema-registry`. See more info in the: awslabs/aws-glue-schema-registry#120
Yep, admittedly I'm piling on here, but this module basically poison-pills any project that has been safely using AWS sdk clients for years if it's added - like with https://github.com/awslabs/amazon-kinesis-client/blob/master/amazon-kinesis-client/src/main/java/software/amazon/kinesis/common/ConfigsBuilder.java#L130-L152 This library is running an entire wrapper of GlueClient.builder()....build() https://github.com/awslabs/aws-glue-schema-registry/blob/master/common/src/main/java/com/amazonaws/services/schemaregistry/common/AWSSchemaRegistryClient.java#L88-L116 is running builders it doesn't need to - it should take in a GlueClient or GlueClient.Builder and finalize it with the modifications it needs. Example code: https://github.com/awslabs/amazon-kinesis-client/blob/master/amazon-kinesis-client/src/main/java/software/amazon/kinesis/common/KinesisClientUtil.java#L40-L49 (KCL making some HTTP configuration settings using builder modifications). Admittedly, stripping the existing use of UrlConnectionHttpClient is a bit of a breaking change for users of this library, however. |
Agree that adding this library shouldn't require exclusions, we will re-evaluate the use of |
#263 |
@dbottini Is there a quick example you can share with us to reproduce the original issue, I wanted to experiment and understand this issue further before reviewing your PR? |
https://github.com/dbottini/schema-registry-client-test |
@blacktooth Is there any chance to prioritize this given the issue has existed since 2021? |
This actually breaks the |
Hi,
I'm getting the error below when integrating schema registry in our project.
I cannot resolve this with the directions above (with our current httpClient), and believe the issue is that
UrlConnectionHttpClient
is used explicitly in the builder below.source
The text was updated successfully, but these errors were encountered: