-
Notifications
You must be signed in to change notification settings - Fork 26
use system props when creating the RestTransport #539
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
base: main
Are you sure you want to change the base?
Changes from all commits
1e0dbaa
312142c
12012ea
f93a619
05f5de9
3f0d9e9
82e2ac6
f0a558e
82be962
6f41ca2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,9 +20,11 @@ | |
| import io.weaviate.client6.v1.internal.rest.DefaultRestTransport; | ||
| import io.weaviate.client6.v1.internal.rest.RestTransport; | ||
| import io.weaviate.client6.v1.internal.rest.RestTransportOptions; | ||
| import lombok.Getter; | ||
|
|
||
| public class WeaviateClient implements AutoCloseable { | ||
| /** Store this for {@link #async()} helper. */ | ||
| @Getter | ||
| private final Config config; | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: replace |
||
| private final RestTransport restTransport; | ||
|
|
@@ -63,14 +65,13 @@ public class WeaviateClient implements AutoCloseable { | |
| public final WeaviateClusterClient cluster; | ||
|
|
||
| public WeaviateClient(Config config) { | ||
| RestTransportOptions restOpt; | ||
| RestTransportOptions restOpt = config.restTransportOptions(); | ||
| GrpcChannelOptions grpcOpt; | ||
| if (config.authentication() == null) { | ||
| restOpt = config.restTransportOptions(); | ||
| grpcOpt = config.grpcTransportOptions(); | ||
| } else { | ||
| TokenProvider tokenProvider; | ||
| try (final var noAuthRest = new DefaultRestTransport(config.restTransportOptions())) { | ||
| try (final var noAuthRest = new DefaultRestTransport(restOpt)) { | ||
| tokenProvider = config.authentication().getTokenProvider(noAuthRest); | ||
| } catch (Exception e) { | ||
| // Generally exceptions are caught in TokenProvider internals. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,7 @@ public CollectionHandle<Map<String, Object>> use( | |
| return use(CollectionDescriptor.ofMap(collectionName), fn); | ||
| } | ||
|
|
||
| private <PropertiesT> CollectionHandle<PropertiesT> use(CollectionDescriptor<PropertiesT> collection, | ||
| public <PropertiesT> CollectionHandle<PropertiesT> use(CollectionDescriptor<PropertiesT> collection, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch |
||
| Function<CollectionHandleDefaults.Builder, ObjectBuilder<CollectionHandleDefaults>> fn) { | ||
| return new CollectionHandle<>(restTransport, grpcTransport, collection, CollectionHandleDefaults.of(fn)); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| package io.weaviate.client6.v1.internal; | ||
|
|
||
| import javax.annotation.Nullable; | ||
|
|
||
| public record Proxy( | ||
| String scheme, | ||
| String host, | ||
| int port, | ||
| @Nullable String username, | ||
| @Nullable String password | ||
| ) { | ||
| public Proxy(String host, int port) { | ||
| this("http", host, port, null, null); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,7 @@ | ||||||||||||||
| package io.weaviate.client6.v1.internal.oidc; | ||||||||||||||
|
|
||||||||||||||
| import io.weaviate.client6.v1.internal.Proxy; | ||||||||||||||
|
|
||||||||||||||
| import java.util.Arrays; | ||||||||||||||
| import java.util.Collections; | ||||||||||||||
| import java.util.HashSet; | ||||||||||||||
|
|
@@ -11,16 +13,36 @@ | |||||||||||||
| public record OidcConfig( | ||||||||||||||
| String clientId, | ||||||||||||||
| String providerMetadata, | ||||||||||||||
| Set<String> scopes) { | ||||||||||||||
| Set<String> scopes, | ||||||||||||||
| OidcProxy proxy) { | ||||||||||||||
|
|
||||||||||||||
| public OidcConfig(String clientId, String providerMetadata, Set<String> scopes) { | ||||||||||||||
| public record OidcProxy( | ||||||||||||||
| String host, | ||||||||||||||
| int port, | ||||||||||||||
| String scheme) { | ||||||||||||||
|
Comment on lines
+19
to
+22
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: similarly to |
||||||||||||||
|
|
||||||||||||||
| public static OidcProxy from(Proxy proxy) { | ||||||||||||||
| return proxy == null ? null : new OidcProxy(proxy.host(), proxy.port(), proxy.scheme()); | ||||||||||||||
| } | ||||||||||||||
|
Comment on lines
+24
to
+26
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Feels like See another comment about the ordering of parameters. |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public OidcConfig(String clientId, String providerMetadata, Set<String> scopes, OidcProxy proxy) { | ||||||||||||||
| this.clientId = clientId; | ||||||||||||||
| this.providerMetadata = providerMetadata; | ||||||||||||||
| this.scopes = scopes != null ? Set.copyOf(scopes) : Collections.emptySet(); | ||||||||||||||
| this.proxy = proxy; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public OidcConfig(String clientId, String providerMetadata, Set<String> scopes) { | ||||||||||||||
| this(clientId, providerMetadata, scopes, null); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public OidcConfig(String clientId, String providerMetadata, List<String> scopes) { | ||||||||||||||
| this(clientId, providerMetadata, scopes == null ? null : new HashSet<>(scopes)); | ||||||||||||||
| this(clientId, providerMetadata, scopes == null ? null : new HashSet<>(scopes), null); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| public OidcConfig(String clientId, String providerMetadata, List<String> scopes, OidcProxy proxy) { | ||||||||||||||
| this(clientId, providerMetadata, scopes == null ? null : new HashSet<>(scopes), proxy); | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| /** Create a new OIDC config with extended scopes. */ | ||||||||||||||
|
|
@@ -31,6 +53,6 @@ public OidcConfig withScopes(String... scopes) { | |||||||||||||
| /** Create a new OIDC config with extended scopes. */ | ||||||||||||||
| public OidcConfig withScopes(List<String> scopes) { | ||||||||||||||
| var newScopes = Stream.concat(this.scopes.stream(), scopes.stream()).collect(Collectors.toSet()); | ||||||||||||||
| return new OidcConfig(clientId, providerMetadata, newScopes); | ||||||||||||||
| return new OidcConfig(clientId, providerMetadata, newScopes, proxy); | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
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 project doesn't use Lombok for generating getters/setters. If you apply the suggestion about
getConfig()inWeaviateClient.javathen this configuration will probably become redundant.