You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by HeartSaVioR <gi...@git.apache.org> on 2017/06/15 05:39:26 UTC

[GitHub] storm pull request #2161: STORM-2556 Break down AutoCreds implementations in...

GitHub user HeartSaVioR opened a pull request:

    https://github.com/apache/storm/pull/2161

    STORM-2556 Break down AutoCreds implementations into two kinds of cla…

    …sses
    
    * Nimbus plugin: implements INimbusCredentialPlugin, ICredentialsRenewer
    * Worker & Topology submitter: implements IAutoCredentials
    * also changed field name for storm-core interfaces to make them clear
      * whether it's cluster conf or topology conf
    
    Please review and comment. Thanks in advance!

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/HeartSaVioR/storm STORM-2556

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/storm/pull/2161.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2161
    
----
commit 3d19b4c40516911658414896e176d92b71bf0ae3
Author: Jungtaek Lim <ka...@gmail.com>
Date:   2017-06-05T23:40:30Z

    STORM-2556 Break down AutoCreds implementations into two kinds of classes
    
    * Nimbus plugin: implements INimbusCredentialPlugin, ICredentialsRenewer
    * Worker & Topology submitter: implements IAutoCredentials
    * also changed field name for storm-core interfaces to make them clear
      * whether it's cluster conf or topology conf

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm pull request #2161: STORM-2556 Break down AutoCreds implementations in...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/storm/pull/2161


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #2161: STORM-2556 Break down AutoCreds implementations into two ...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/2161
  
    @revans2 
    Totally agreed. Currently it's tightly coupled with Hadoop security but is not represented in abstract class name. I'll see whether we can extract non-Hadoop things from current abstract base classes, and rename current classes to represent Hadoop.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #2161: STORM-2556 Break down AutoCreds implementations into two ...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/2161
  
    @revans2 
    I renamed abstract classes to have 'Hadoop' on their name.
    Btw I don't see other cases to not using Hadoop, hence not sure that generalization provides some benefits for now. If you see some benefits, let's file an issue and track it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #2161: STORM-2556 Break down AutoCreds implementations into two ...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:

    https://github.com/apache/storm/pull/2161
  
    @revans2 OK. Got it. Thanks for reviewing!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #2161: STORM-2556 Break down AutoCreds implementations into two ...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:

    https://github.com/apache/storm/pull/2161
  
    @HeartSaVioR I don't think more generalization is a benefit.  My comment was mostly around making it hadoop specific or truly making it generic.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] storm issue #2161: STORM-2556 Break down AutoCreds implementations into two ...

Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:

    https://github.com/apache/storm/pull/2161
  
    I am fine with splitting things up, but I am not super excited about how we setup the Abstract* classes.  Perhaps it is just how we are naming them, because there are a lot of things in them that are very Hadoop specific, aka UserGroupInformation.  Could we at least rename the current set of Abstract base classes to have Hadoop somewhere in the name?  If you want to go overboard we could look at truly providing a generic base class for credentials that has pluggable pieces for pulling out the raw string credentials (similar to CredentialKeyProvider), deserializing them, and putting them in the Subject correctly.  Then on the other side getting the credentials, serializing them to a string, and finally putting them into the string credentials object (again similar to CredentialKeyProvider).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---