-
Notifications
You must be signed in to change notification settings - Fork 45
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-145 SQL explain drives rest flow to OIDC endpoint. #146
Conversation
@grzegorz8 @kristoffSC could you review this one please? |
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.
LGTM,
only small comments
src/main/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClient.java
Outdated
Show resolved
Hide resolved
src/main/java/com/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClient.java
Outdated
Show resolved
Hide resolved
...om/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientWithWireTest.java
Show resolved
Hide resolved
src/test/java/com/getindata/connectors/http/internal/utils/HttpHeaderUtilsTest.java
Outdated
Show resolved
Hide resolved
...om/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientWithWireTest.java
Outdated
Show resolved
Hide resolved
...om/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientWithWireTest.java
Outdated
Show resolved
Hide resolved
...om/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientWithWireTest.java
Outdated
Show resolved
Hide resolved
...om/getindata/connectors/http/internal/table/lookup/JavaNetHttpPollingClientWithWireTest.java
Outdated
Show resolved
Hide resolved
Signed-off-by: davidradl <[email protected]>
Great thanks @grzegorz8 and @kristoffSC for you prompt reviews - I have made the changes you suggest. Let me know if you see anything more or LGTM! |
@grzegorz8 and @kristoffSC it would be great for us if we could get this merged and a release created quickly. Many thanks. |
Discussions have been resolved. Dismissing review to unblock merge.
Hey! Done. I tried yesterday a few times, but uploads failed due to timeouts. Fortunately, publish pipeline succeeded today. |
Description
Change the code so that the OIDC authentication flow is moved to lookup query time. This means that explain select - does not issue a network call.
Resolves http145
PR Checklist