You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@airavata.apache.org by anujbhan <gi...@git.apache.org> on 2016/10/10 22:48:31 UTC
[GitHub] airavata pull request #54: Adding SSH Credential summary data model
GitHub user anujbhan opened a pull request:
https://github.com/apache/airavata/pull/54
Adding SSH Credential summary data model
This feature will help retrieve more information on SSH public keys displayed in gateway Clients.
**Airavata Endpoint affected:**
**New:**
```public List<CredentialSummary> getAllGatewaySSHPubKeysSummary(AuthzToken authzToken, String gatewayId)```
**returns a list of Credential objects :**
```struct CredentialSummary {
1: required string gatewayId,
2: required string username,
3: optional string publicKey,
4: optional i64 persistedTime,
5: optional string description
}```
**Note: API level test are pending, will push code soon**
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/anujbhan/airavata ssh-cred-model-change
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/airavata/pull/54.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 #54
----
commit eb0cd635ea9e065d622abb30ff30b54a9f60910b
Author: Anuj Bhandar <bh...@gmail.com>
Date: 2016-10-10T16:58:25Z
Adding SSHCredentialSummary model & adding desc field to SSHCredential
commit 3170fefdf371e6db6cd4099570ff294909eec10e
Author: Anuj Bhandar <bh...@gmail.com>
Date: 2016-10-10T19:18:02Z
Making the token field required in SSHCredentialSummary
commit efed2956fb93443575da73db0128ba1b3a18416d
Author: Anuj Bhandar <bh...@gmail.com>
Date: 2016-10-10T20:34:38Z
removing add and delete method for SSHCredentialSummary, serverhandler implemented got get method
commit 218870c9da84ff5ce6cc8a7bfa6dfaa34a025553
Author: Anuj Bhandar <bh...@gmail.com>
Date: 2016-10-10T22:27:52Z
adding method to get all ssh pub key Summaries
commit fcc44af33b34afa230a190e92a9b3f80123dadd9
Author: Anuj Bhandar <bh...@gmail.com>
Date: 2016-10-10T22:30:04Z
implementing DAO and db utils
commit b6548366daab592e260acbd3fb77001986866678
Author: Anuj Bhandar <bh...@gmail.com>
Date: 2016-10-10T22:33:41Z
Implementing API level changes
----
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by anujbhan <gi...@git.apache.org>.
Github user anujbhan commented on the issue:
https://github.com/apache/airavata/pull/54
@shamrath,
Indeed a good list of improvements, this way the Airavata Api will remain clutter free. I will start the development on these shortly.
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by anujbhan <gi...@git.apache.org>.
Github user anujbhan commented on the issue:
https://github.com/apache/airavata/pull/54
@smarru,
Method to fetch specific user's public keys with summary is added
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by anujbhan <gi...@git.apache.org>.
Github user anujbhan commented on the issue:
https://github.com/apache/airavata/pull/54
@shamrath,
Here is the follow up JIRA : https://issues.apache.org/jira/browse/AIRAVATA-2163, and I have moved the credential store thrift file to data-models directory, though the actual data models will be created in credential-store modules directory.
---
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] airavata pull request #54: Adding SSH Credential summary data model
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/airavata/pull/54
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by anujbhan <gi...@git.apache.org>.
Github user anujbhan commented on the issue:
https://github.com/apache/airavata/pull/54
All the festures requested are there in this PR. But i am working on the
additional changes prescribed by shameera
On Oct 12, 2016 2:16 PM, "Suresh Marru" <no...@github.com> wrote:
Can we review and merge this now? I mean are you done working on this PR?
> On Oct 12, 2016, at 1:19 PM, Anuj Bhandar <no...@github.com>
wrote:
>
> @smarru <https://github.com/smarru>,
> Method to fetch specific user's public keys with summary is added
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <
https://github.com/apache/airavata/pull/54#issuecomment-253278638>, or mute
the thread <https://github.com/notifications/unsubscribe-
auth/ACbXSjo-r85VddHayUhJa-y_SYvfh7oPks5qzRa4gaJpZM4KTEWl>.
>
\u2014
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<https://github.com/apache/airavata/pull/54#issuecomment-253294073>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AOOY4bhzay3sRrxXSSXKQWcEbqqkG-u8ks5qzSQHgaJpZM4KTEWl>
.
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by smarru <gi...@git.apache.org>.
Github user smarru commented on the issue:
https://github.com/apache/airavata/pull/54
Very nice list of improvements. I was penciling some of these for a API review, but good that you are suggesting to get them done now.
+1 for all 4 suggestions.
Suresh
> On Oct 10, 2016, at 10:13 PM, Shameera <no...@github.com> wrote:
>
> Hi Anuj
>
> Followings are few suggestions to improve your pull request.
>
> We can do this in a better way by adding generic API method(to AiravataAPIServer) like "getAllCredentialSummary" instead of adding API method for each credential types eg: "getAllGatewaySSHPubKeysSummary" which will return all the credential summary for that particular gatewayId. You may be able to remove few redundant methods from Airavata API server.
> Let's introduced one credential summary struct to all credential types, how we differentiate is we have credential type enum field( SSH, PASSWD, CERT etc ) in the credential summary thrift struct.
> Move credential data model thrift file to data model directory and merge credential summary thrift file with it.
> Credential stubs still have date fields in generated thrift files, not because your changes, but we need to fix it so remove it.
> \u2014
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub <https://github.com/apache/airavata/pull/54#issuecomment-252793313>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACbXSnb1A4mTShyGtBq_KbeBSM8VGq-Vks5qyvCxgaJpZM4KTEWl>.
>
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by shamrath <gi...@git.apache.org>.
Github user shamrath commented on the issue:
https://github.com/apache/airavata/pull/54
@anujbhan ,
Ok let's make this an exception. Please create follow up JIRA to track the pending improvements. I will review and merge this.
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by anujbhan <gi...@git.apache.org>.
Github user anujbhan commented on the issue:
https://github.com/apache/airavata/pull/54
@smarru & @shamrath,
Optimization to this PR may take some time. Since this PR is critical, Please consider merging. I will submit the changes as soon as possible with another pull request.
The build is passing in my local environment, don't know why Travis is failing.
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by smarru <gi...@git.apache.org>.
Github user smarru commented on the issue:
https://github.com/apache/airavata/pull/54
Can we review and merge this now? I mean are you done working on this PR?
> On Oct 12, 2016, at 1:19 PM, Anuj Bhandar <no...@github.com> wrote:
>
> @smarru <https://github.com/smarru>,
> Method to fetch specific user's public keys with summary is added
>
> \u2014
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub <https://github.com/apache/airavata/pull/54#issuecomment-253278638>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACbXSjo-r85VddHayUhJa-y_SYvfh7oPks5qzRa4gaJpZM4KTEWl>.
>
---
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] airavata issue #54: Adding SSH Credential summary data model
Posted by shamrath <gi...@git.apache.org>.
Github user shamrath commented on the issue:
https://github.com/apache/airavata/pull/54
Hi Anuj
Followings are few suggestions to improve your pull request.
1. We can do this in a better way by adding generic API method(to AiravataAPIServer) like "getAllCredentialSummary" instead of adding API method for each credential types eg: "getAllGatewaySSHPubKeysSummary" which will return all the credential summary for that particular gatewayId. You may be able to remove few redundant methods from Airavata API server.
2. Let's introduced one credential summary struct to all credential types, how we differentiate is we have credential type enum field( SSH, PASSWD, CERT etc ) in the credential summary thrift struct.
3. Move credential data model thrift file to data model directory and merge credential summary thrift file with it.
4. Credential stubs still have date fields in generated thrift files, not because your changes, but we need to fix it so remove 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.
---