cgivre commented on code in PR #2668:
URL: https://github.com/apache/drill/pull/2668#discussion_r985723123
##########
contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java:
##########
@@ -832,8 +835,36 @@ public static List<String>
buildParameterList(NullableVarCharHolder[] inputReade
return inputArguments;
}
- public static HttpApiConfig getEndpointConfig(String endpoint,
HttpStoragePluginConfig pluginConfig) {
- HttpApiConfig endpointConfig = pluginConfig.getConnection(endpoint);
+ /**
+ * This function is used to obtain the configuration information for a given
API in the HTTP UDF.
+ * If aliasing is enabled, this function will resolve aliases for
connections.
+ * @param endpoint The name of the endpoint. Should be a {@link String}
+ * @param context The {@link DrillbitContext} from the current query.
+ * @param info {@link ContextInformation} from the current query.
+ * @param pluginConfig The {@link HttpStoragePluginConfig} the configuration
from the plugin
+ * @return The {@link HttpApiConfig} corresponding with the endpoint.
+ */
+ public static HttpApiConfig getEndpointConfig(String endpoint,
+ DrillbitContext context,
+ ContextInformation info,
+ HttpStoragePluginConfig
pluginConfig) {
+ String queryUser = info.getQueryUser();
+ AliasRegistryProvider aliasRegistryProvider =
context.getAliasRegistryProvider();
Review Comment:
>
To my knowledge, this is the only UDF that accesses the storage layer. I
was thinking the same thing however, but if the user doesn't know that aliasing
is being used, they'd get a lot of strange errors and not necessarily know what
to do.
##########
contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/util/SimpleHttp.java:
##########
@@ -832,8 +835,36 @@ public static List<String>
buildParameterList(NullableVarCharHolder[] inputReade
return inputArguments;
}
- public static HttpApiConfig getEndpointConfig(String endpoint,
HttpStoragePluginConfig pluginConfig) {
- HttpApiConfig endpointConfig = pluginConfig.getConnection(endpoint);
+ /**
+ * This function is used to obtain the configuration information for a given
API in the HTTP UDF.
+ * If aliasing is enabled, this function will resolve aliases for
connections.
+ * @param endpoint The name of the endpoint. Should be a {@link String}
+ * @param context The {@link DrillbitContext} from the current query.
+ * @param info {@link ContextInformation} from the current query.
+ * @param pluginConfig The {@link HttpStoragePluginConfig} the configuration
from the plugin
+ * @return The {@link HttpApiConfig} corresponding with the endpoint.
+ */
+ public static HttpApiConfig getEndpointConfig(String endpoint,
+ DrillbitContext context,
+ ContextInformation info,
+ HttpStoragePluginConfig
pluginConfig) {
+ String queryUser = info.getQueryUser();
+ AliasRegistryProvider aliasRegistryProvider =
context.getAliasRegistryProvider();
Review Comment:
> Good point regarding splitting the logic. I would propose to move it
instead of UDF to another place, before the `http_request` is evaluated, for
example to `RexToDrill.getDrillFunctionFromOptiqCall` (or another location), so
it could have a consistent behavior for regular aliases and aliases for this
function when query plans contain actual storage name instead of the aliases.
I like the idea. I was almost thinking of doing it in the
`StoragePluginRegistry`. What do you think? Could we make that a separate
JIRA as I imagine that is considerably more complex?
I should mention that this will still work if there are no aliases. The
order is first check to see if there is a user alias, if not, then check to see
if there is a public alias, and finally if neither, use the actual text.
--
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: dev-unsubscr...@drill.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
