You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "shalk (via GitHub)" <gi...@apache.org> on 2023/03/14 11:55:31 UTC

[GitHub] [incubator-pegasus] shalk opened a new pull request, #1393: fix jaas conf; make thread daemon

shalk opened a new pull request, #1393:
URL: https://github.com/apache/incubator-pegasus/pull/1393

   ### What problem does this PR solve? <!--add issue link with summary if exists-->
   
   fix #1389 
   
   ### What is changed and how does it work?
   
   
   ### Checklist <!--REMOVE the items that are not applicable-->
   
   ##### Tests <!-- At least one of them must be included. -->
   
   - Unit test
   - Integration test
   - Manual test (add detailed scripts or steps below)
   - No code
   
   ##### Code changes
   
   - Has exported function/method change
   - Has exported variable/fields change
   - Has interface methods change
   - Has persistent data change
   
   ##### Side effects
   
   - Possible performance regression
   - Increased code complexity
   - Breaking backward compatibility
   
   ##### Related changes
   
   - Need to cherry-pick to the release branch
   - Need to update the documentation
   - Need to be included in the release note
   


-- 
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-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] empiredan merged pull request #1393: fix: jaas conf; make thread daemon

Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan merged PR #1393:
URL: https://github.com/apache/incubator-pegasus/pull/1393


-- 
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-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] Apache9 commented on a diff in pull request #1393: fix: jaas conf; make thread daemon

Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #1393:
URL: https://github.com/apache/incubator-pegasus/pull/1393#discussion_r1136848064


##########
java-client/src/main/java/org/apache/pegasus/security/KerberosProtocol.java:
##########
@@ -53,7 +54,18 @@ class KerberosProtocol implements AuthProtocol {
   private String keyTab;
   private String principal;
   final int CHECK_TGT_INTEVAL_SECONDS = 10;
-  final ScheduledExecutorService service = Executors.newSingleThreadScheduledExecutor();
+  final ScheduledExecutorService service =
+      Executors.newSingleThreadScheduledExecutor(new TgtThreadFactory());
+
+  static class TgtThreadFactory implements ThreadFactory {

Review Comment:
   Just create an anonymous class is enough?



##########
java-client/src/main/java/org/apache/pegasus/security/KerberosProtocol.java:
##########
@@ -161,19 +173,16 @@ private static Configuration getLoginContextConfiguration(String keyTab, String
       @Override
       public AppConfigurationEntry[] getAppConfigurationEntry(String name) {
         Map<String, String> options = new HashMap<>();
-        // TGT is obtained from the ticket cache.
-        options.put("useTicketCache", "true");

Review Comment:
   This is for keytab based login so we should remove useTicketCache, but do we support login use ticket cache? There is another method for ticker cache based login?



-- 
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-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] shalk commented on a diff in pull request #1393: fix: jaas conf; make thread daemon

Posted by "shalk (via GitHub)" <gi...@apache.org>.
shalk commented on code in PR #1393:
URL: https://github.com/apache/incubator-pegasus/pull/1393#discussion_r1138386194


##########
java-client/src/main/java/org/apache/pegasus/security/KerberosProtocol.java:
##########
@@ -161,19 +173,16 @@ private static Configuration getLoginContextConfiguration(String keyTab, String
       @Override
       public AppConfigurationEntry[] getAppConfigurationEntry(String name) {
         Map<String, String> options = new HashMap<>();
-        // TGT is obtained from the ticket cache.
-        options.put("useTicketCache", "true");

Review Comment:
   yes, we can use ticket cache without keytab.  these can be done in the same method like hadoop



-- 
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-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org