cgivre commented on code in PR #2547:
URL: https://github.com/apache/drill/pull/2547#discussion_r875147774
##########
exec/java-exec/src/main/java/org/apache/drill/exec/server/rest/OAuthRequests.java:
##########
@@ -121,26 +106,19 @@ public static Response updateRefreshToken(String name,
OAuthTokenContainer token
public static Response updateOAuthTokens(String name, OAuthTokenContainer
tokenContainer, StoragePluginRegistry storage,
UserAuthEnabled authEnabled,
SecurityContext sc) {
try {
- if (storage.getPlugin(name).getConfig() instanceof
CredentialedStoragePluginConfig) {
Review Comment:
@jnturton Thanks for this PR. These functions used to have some checks to
see if the storage plugin in question was using OAuth. The prerequisite was
that the plugin used the `CredentialedStoragePluginConfig`.
Now that is no longer an option, do you think it's worth adding some check
to see if the plugin is in fact using OAuth? If not, throw some sort of error
saying you can't update tokens on a non-OAuth plugin?
In theory a user would never really be given that choice because the UI
takes care of that for them so I'm not sure if it is really necessary or not.
--
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]
