kingswanwho commented on code in PR #2499:
URL: https://github.com/apache/drill/pull/2499#discussion_r968518305
##########
exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonation.java:
##########
@@ -156,22 +159,25 @@ public void unauthorizedTarget() throws Exception {
@Test
public void invalidPolicy() throws Exception {
- thrownException.expect(new
UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION,
- "Invalid impersonation policies."));
+ String query = "ALTER SYSTEM SET `%s`='%s'";
Review Comment:
@jnturton Hi James, the **_client_** here inherits from BaseTestQuery, which
is DrillClient but not ClientFixture in TestLargeFileCompilation as Paul
suggested. DrillClient doesn't have method like client.alterSystem().
BaseTestQuery is deprecated actually, but not marked as deprecated because it
is still widely used. The best choice here I think is change base test class
from BaseTestQuery to ClusterTest so that we could use client.alterSystem() for
clean and compact code style. There are multiple Inheritance levels here, which
are TestInboundImpersonation -> BaseTestImpersonation -> PlanTestBase ->
BaseTestQuery. Due to PlanTestBase involves many classes. Maybe change
BaseTestImpersonation inheritance from PlanTestBase to ClusterTest directly is
an easier way. However, this PR mainly focus on the deprecated clean up, but
induces more modifications on UT. Is this an appropriate change here?
--
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
