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

Apps should explicitly import java.net.http.HttpClient instances of RegistryLookup #241

Closed
istreeter opened this issue May 18, 2023 · 0 comments

Comments

@istreeter
Copy link
Contributor

Until now, we have provided a "Sync" instance and a "Id" instance of a RegistryLookup in the companion object, which are based on java.net.http.HttpClient. This means that an implicit instance of RegistryLookup is automatically in scope whenever there is an instance of a Sync. This was easy for users consuming this library, because they didn't need to do an extra import to put the RegistryLookup in scope.

However.... more recently we are transitioning to favour the http4s implementation of the RegistryLookup. This lookup needs to be provided explicitly by the application; there is no automatic summoning of an implicit instance.

The problem is... if a developer forgets to provide the http4s implicit instance AND if the app has an instance of Sync[F] then it's all too easy to accidentally use the java.net.http lookup instead of the intended http4s lookup.

My suggestion is to move the standard lookups into a new object called JavaNetRegistryLookup. This means that users must always explicitly import or provide the lookup they want to use. There is no chance of accidentally using the wrong lookup.

We are soon planning to release iglu-scala-client version 3.0.0, so now is a good time to make this breaking change. Applications upgrading to 3.0.0 must either use the http4s lookup, or add a line like this to their imports:

import com.snowplowanalytics.iglu.client.resolver.registries.JavaNetRegistryLookup._
istreeter added a commit that referenced this issue May 18, 2023
istreeter added a commit that referenced this issue May 18, 2023
istreeter added a commit that referenced this issue May 25, 2023
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

No branches or pull requests

1 participant