bharathgunapati commented on code in PR #58:
URL:
https://github.com/apache/flink-connector-http/pull/58#discussion_r4371930794
##########
docs/content/docs/connectors/table/http.md:
##########
@@ -266,7 +266,7 @@ Note the options with the prefix _http_ are the HTTP
connector specific options,
| http.security.cert.server.allowSelfSigned |
optional | Accept untrusted certificates for TLS communication.
|
| http.security.oidc.token.request |
optional | OIDC `Token Request` body in `application/x-www-form-urlencoded`
encoding
|
| http.security.oidc.token.endpoint.url |
optional | OIDC `Token Endpoint` url, to which the token request will be issued
|
-| http.security.oidc.token.expiry.reduction |
optional | OIDC tokens will be requested if the current time is later than the
cached token expiry time minus this value.
|
+| http.security.oidc.token.expiry.reduction |
optional | OIDC tokens will be requested if the current time is later than the
cached token expiry time minus this value. The default is `30s`.
|
Review Comment:
You are right that the old default was 1 second in both the code and the
docs. I would like to keep 30 seconds. Here is why.
Before this PR the 1 second default did nothing. The lookup fetched a new
token on every request, so no token ever lived long enough to get close to its
expiry. Now tokens are cached, so this setting is what protects the last
requests before a token expires. With 1 second, a request sent just before the
cutoff can reach the server after the token has already expired. One second is
not enough to cover the request time, the token endpoint time, and clock
differences between Flink and the identity provider. The user then gets a 401
that they cannot explain from their config. 30 seconds is the usual value in
OIDC clients.
The cost is small. A token is refreshed 29 seconds earlier than before.
Tokens usually live for 5 to 60 minutes, so the extra token calls are
negligible. Jobs that set `http.security.oidc.token.expiry.reduction`
themselves are not affected. If a token's `expires_in` is 30 seconds or less,
the full lifetime is used, so short test tokens still work. The change is
listed in the compatibility note in the PR description.
If you still prefer 1 second in this PR, I will revert it and open a
follow-up Jira for the default. But I think users are better off with 30
seconds from the first release where caching works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
