jihoonson commented on code in PR #12396:
URL: https://github.com/apache/druid/pull/12396#discussion_r853714360
##########
integration-tests/src/test/java/org/apache/druid/tests/security/AbstractAuthConfigurationTest.java:
##########
@@ -188,22 +199,23 @@
protected HttpClient adminClient;
protected HttpClient datasourceOnlyUserClient;
+ protected HttpClient datasourceAndContextParamsClient;
protected HttpClient datasourceAndSysUserClient;
protected HttpClient datasourceWithStateUserClient;
protected HttpClient stateOnlyUserClient;
protected HttpClient internalSystemClient;
protected abstract void setupDatasourceOnlyUser() throws Exception;
+ protected abstract void setupDatasourceAndContextParamsUser() throws
Exception;
protected abstract void setupDatasourceAndSysTableUser() throws Exception;
protected abstract void setupDatasourceAndSysAndStateUser() throws Exception;
protected abstract void setupSysTableAndStateOnlyUser() throws Exception;
protected abstract void setupTestSpecificHttpClients() throws Exception;
protected abstract String getAuthenticatorName();
protected abstract String getAuthorizerName();
protected abstract String getExpectedAvaticaAuthError();
- protected abstract Properties getAvaticaConnectionProperties();
Review Comment:
It was my bad that I removed this method before. I missed that
implementations of this class can have a different implementation for this
method. I added them back.
for `testAvaticaQuery`, I think it's OK to change the method signature as
the new signature makes it clearer what user's credentials are used for the
test.
##########
server/src/main/java/org/apache/druid/server/QueryResource.java:
##########
@@ -364,7 +341,12 @@ public void write(OutputStream outputStream) throws
WebApplicationException
log.noStackTrace()
.makeAlert(e, "Exception handling request")
- .addData("query", query != null ?
jsonMapper.writeValueAsString(query) : "unparseable query")
Review Comment:
This is an intentional change. It doesn't seem like a good convention to use
an object after the object is moved to inside of another. Note that `Query` is
immutable and you need to create a new copy when you annotate it with extra
information. Even though this is not currently happening in `QueryLifecycle`,
it might be able to happen later in the future. Then, the `query` in
`QueryResource` will not be guaranteed to be the same object as the `query` in
`QueryLifecycle`.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
