You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by Ignasi Barrera <no...@github.com> on 2013/09/12 23:43:10 UTC
[jclouds-chef] Cleaned up ChefApi and ChefService interfaces (#19)
This pull request just renames a few methods in the ChefApi to follow a consistent naming convention, and reorders them so all methods are properly grouped. There are no changes to the code just the bits to follow the more convenient naming convention.
Also cleaned up the ChefService and improved the javadoc on the methods of both interfaces.
@noorul, could you take this PR into account when adding new features to the API?
You can merge this Pull Request by running:
git pull https://github.com/nacx/jclouds-chef interfaces
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-chef/pull/19
-- Commit Summary --
* Cleaned up ChefApi and ChefService interfaces
-- File Changes --
M core/src/main/java/org/jclouds/chef/ChefApi.java (1078)
M core/src/main/java/org/jclouds/chef/ChefApiMetadata.java (7)
M core/src/main/java/org/jclouds/chef/ChefService.java (194)
M core/src/main/java/org/jclouds/chef/filters/SignedHeaderAuth.java (4)
M core/src/main/java/org/jclouds/chef/internal/BaseChefService.java (153)
M core/src/main/java/org/jclouds/chef/strategy/internal/ListCookbookVersionsImpl.java (2)
M core/src/main/java/org/jclouds/chef/strategy/internal/ListEnvironmentNodesImpl.java (4)
M core/src/main/java/org/jclouds/chef/test/TransientChefApi.java (20)
M core/src/test/java/org/jclouds/chef/ChefApiExpectTest.java (20)
M core/src/test/java/org/jclouds/chef/ChefApiTest.java (100)
M core/src/test/java/org/jclouds/chef/binders/BindHexEncodedMD5sToJsonPayloadTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/GroupToBootScriptTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseClientFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseCookbookDefinitionFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseCookbookDefinitionFromJsonv10Test.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseCookbookDefinitionListFromJsonv10Test.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseCookbookVersionFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseCookbookVersionsV09FromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseCookbookVersionsV10FromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseDataBagItemFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseKeySetFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseNodeFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseSandboxFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseSearchDataBagItemFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/ParseUploadSandboxFromJsonTest.java (4)
M core/src/test/java/org/jclouds/chef/functions/RunListForGroupTest.java (3)
M core/src/test/java/org/jclouds/chef/internal/BaseChefApiLiveTest.java (18)
M core/src/test/java/org/jclouds/chef/internal/BaseStubbedOhaiLiveTest.java (2)
M core/src/test/java/org/jclouds/ohai/config/JMXTest.java (4)
M core/src/test/java/org/jclouds/ohai/config/OhaiModuleTest.java (6)
M core/src/test/java/org/jclouds/ohai/functions/NestSlashKeysTest.java (4)
M enterprise/src/main/java/org/jclouds/enterprisechef/EnterpriseChefApiMetadata.java (3)
M enterprise/src/test/java/org/jclouds/enterprisechef/EnterpriseChefApiExpectTest.java (22)
M enterprise/src/test/java/org/jclouds/enterprisechef/EnterpriseChefProviderMetadataTest.java (2)
M enterprise/src/test/java/org/jclouds/enterprisechef/binders/BindGroupToUpdateRequestJsonPayloadTest.java (4)
M enterprise/src/test/java/org/jclouds/enterprisechef/binders/GroupNameTest.java (1)
-- Patch Links --
https://github.com/jclouds/jclouds-chef/pull/19.patch
https://github.com/jclouds/jclouds-chef/pull/19.diff
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Andrew Phillips <no...@github.com>.
> throw new UnsupportedOperationException();
> }
>
> @Override
> - public CookbookDefinition getEnvironmentCookbook(String environmentname, String cookbookname) {
`cookbookName` or cookbookname`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19/files#r6363858
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by BuildHive <no...@github.com>.
[jclouds ยป jclouds-chef #374](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-chef/374/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19#issuecomment-24358620
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Andrew Phillips <no...@github.com>.
>
> + /**
> + * Delete the given nodes.
> + *
> + * @param names The names of the nodes to delete.
> + */
> + void deleteAllNodesInList(Iterable<String> names);
> +
> + /**
> + * Delete the given clients.
> + *
> + * @param names The names of the client to delete.
> + */
> void deleteAllClientsInList(Iterable<String> names);
Perhaps than simply make the doc comment `// Nodes/Clients` or so..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19/files#r6371238
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Andrew Phillips <no...@github.com>.
> @@ -44,6 +44,11 @@
> * @author Ignasi Barrera
> */
> public class ChefApiMetadata extends BaseHttpApiMetadata<ChefApi> {
> +
> + /**
> + * The default Chef Server API version to use.
> + */
> + public static final String DEFAULT_VERSION = "0.10.8";
`DEFAULT_API_VERSION`? Just wondering whether otherwise it could be confused with the version _of the metadata_?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19/files#r6363859
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-chef-pull-requests #45](https://jclouds.ci.cloudbees.com/job/jclouds-chef-pull-requests/45/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19#issuecomment-24358685
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Andrew Phillips <no...@github.com>.
>
> + /**
> + * Delete the given nodes.
> + *
> + * @param names The names of the nodes to delete.
> + */
> + void deleteAllNodesInList(Iterable<String> names);
> +
> + /**
> + * Delete the given clients.
> + *
> + * @param names The names of the client to delete.
> + */
> void deleteAllClientsInList(Iterable<String> names);
Nodes and clients belong in the same block?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19/files#r6363855
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Ignasi Barrera <no...@github.com>.
Agree with the comments, will open a PR in short to address them :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19#issuecomment-24493060
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Andrew Phillips <no...@github.com>.
Thanks for the cleanup, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19#issuecomment-24446875
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Ignasi Barrera <no...@github.com>.
>
> + /**
> + * Delete the given nodes.
> + *
> + * @param names The names of the nodes to delete.
> + */
> + void deleteAllNodesInList(Iterable<String> names);
> +
> + /**
> + * Delete the given clients.
> + *
> + * @param names The names of the client to delete.
> + */
> void deleteAllClientsInList(Iterable<String> names);
I feel so. For every node registered in a Chef server there is also a client. The node holds the information of the host itself, and the client is the "user" the node uses to access the api, and one is created for each registered node. Although there can be clients created to provide access to the api to other apps, the common use case is to have node-client pairs and treat them as a single entity.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19/files#r6371143
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Andrew Phillips <no...@github.com>.
>
> /**
> - * deletes an existing cookbook.
> + * List the names of the existing clients.
Don't quite know whether we have a jclouds convention, but according to the general Javadoc style guide this should probably be "Lists the names..."? Similarly for other methods:
["Use 3rd person (descriptive) not 2nd person (prescriptive)."](http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19/files#r6363843
Re: [jclouds-chef] Cleaned up ChefApi and ChefService interfaces
(#19)
Posted by Andrew Phillips <no...@github.com>.
> @@ -343,22 +343,22 @@ public Environment updateEnvironment(Environment environment) {
> }
>
> @Override
> - public Set<CookbookDefinition> listEnvironmentCookbooks(String environmentname) {
> + public Set<CookbookDefinition> listCookbooksInEnvironment(String environmentname) {
Any reason for `databagName` above but `environmentname` (lowercase 'n') here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-chef/pull/19/files#r6363856