You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2021/10/28 15:48:59 UTC
[GitHub] [accumulo] milleruntime opened a new pull request #2333: Drop second RPC call after shutdown in ManagerApiIT
milleruntime opened a new pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333
* Closes #2327
* Drop the second RPC call in the last test of the ManagerApiIT
* ThriftTransportPool is now a part of ClientContext and only allows for
a single remaining connection after shutting down
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on pull request #2333: Drop second RPC call after shutdown in ManagerApiIT
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333#issuecomment-953990494
Actually, I think I figured out a way to do both calls in the test after shutting down. Putting up a change shortly.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] Manno15 commented on pull request #2333: Modify shutdown test in ManagerApiIT
Posted by GitBox <gi...@apache.org>.
Manno15 commented on pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333#issuecomment-954010695
> The IT is passing with my last changes:
I can confirm the test is passing now.
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #2333: Modify shutdown test in ManagerApiIT
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] ctubbsii commented on a change in pull request #2333: Modify shutdown test in ManagerApiIT
Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333#discussion_r739330977
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java
##########
@@ -210,12 +210,19 @@ public void testPermissions_shutdownTabletServer() throws Exception {
// see the junit annotation to control test ordering at the top of this class
@Test
public void z99_testPermissions_shutdown() throws Exception {
+ // grab connections before shutting down
+ AccumuloClient rootUserClient = Accumulo.newClient().from(getClientProps())
+ .as(rootUser.getPrincipal(), rootUser.getToken()).build();
+ AccumuloClient privilegedUserClient = Accumulo.newClient().from(getClientProps())
+ .as(privilegedUser.getPrincipal(), privilegedUser.getToken()).build();
// To shutdown, user needs SystemPermission.SYSTEM
op = user -> client -> client.shutdown(null, user, false);
expectPermissionDenied(op, regularUser);
// We should be able to do both of the following RPC calls before it actually shuts down
- expectPermissionSuccess(op, rootUser);
- expectPermissionSuccess(op, privilegedUser);
+ expectPermissionSuccess(op, (ClientContext) rootUserClient);
+ expectPermissionSuccess(op, (ClientContext) privilegedUserClient);
+ rootUserClient.close();
+ privilegedUserClient.close();
Review comment:
These clients should be in try-with-resources blocks, rather than have explicit calls to `close()`
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on a change in pull request #2333: Modify shutdown test in ManagerApiIT
Posted by GitBox <gi...@apache.org>.
milleruntime commented on a change in pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333#discussion_r739475917
##########
File path: test/src/main/java/org/apache/accumulo/test/functional/ManagerApiIT.java
##########
@@ -210,12 +210,19 @@ public void testPermissions_shutdownTabletServer() throws Exception {
// see the junit annotation to control test ordering at the top of this class
@Test
public void z99_testPermissions_shutdown() throws Exception {
+ // grab connections before shutting down
+ AccumuloClient rootUserClient = Accumulo.newClient().from(getClientProps())
+ .as(rootUser.getPrincipal(), rootUser.getToken()).build();
+ AccumuloClient privilegedUserClient = Accumulo.newClient().from(getClientProps())
+ .as(privilegedUser.getPrincipal(), privilegedUser.getToken()).build();
// To shutdown, user needs SystemPermission.SYSTEM
op = user -> client -> client.shutdown(null, user, false);
expectPermissionDenied(op, regularUser);
// We should be able to do both of the following RPC calls before it actually shuts down
- expectPermissionSuccess(op, rootUser);
- expectPermissionSuccess(op, privilegedUser);
+ expectPermissionSuccess(op, (ClientContext) rootUserClient);
+ expectPermissionSuccess(op, (ClientContext) privilegedUserClient);
+ rootUserClient.close();
+ privilegedUserClient.close();
Review comment:
I spent way too much time getting the formatting to not be hideous so I hope you appreciate the double try-with-resources in 1f0bcd1 :stuck_out_tongue_winking_eye:
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime commented on pull request #2333: Drop second RPC call after shutdown in ManagerApiIT
Posted by GitBox <gi...@apache.org>.
milleruntime commented on pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333#issuecomment-954006915
The IT is passing with my last changes:
<pre>
[INFO] Running org.apache.accumulo.test.functional.ManagerApiIT
[INFO] Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 19.635 s - in org.apache.accumulo.test.functional.ManagerApiIT
</pre>
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [accumulo] milleruntime merged pull request #2333: Modify shutdown test in ManagerApiIT
Posted by GitBox <gi...@apache.org>.
milleruntime merged pull request #2333:
URL: https://github.com/apache/accumulo/pull/2333
--
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: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org