You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jclouds.apache.org by carlosgarciaibanez <no...@github.com> on 2013/07/30 13:55:59 UTC
[jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure to
limit enterprises to datacenters (#17)
The datacenter link has been added to the Limits object construction. This link is used in the 2.4 version of the Abiquo API to establish a relation between an enterprise and the datacenter it is restricted to.
You can merge this Pull Request by running:
git pull https://github.com/carlosgarciaibanez/jclouds-labs master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds-labs/pull/17
-- Commit Summary --
* JCLOUDS-208: The datacenter link has been added to the Limits object construction. This link is used in the 2.4 version of the Abiquo API to establish a relation between an enterprise and the datacenter it is restricted to
-- File Changes --
M abiquo/src/main/java/org/jclouds/abiquo/domain/enterprise/Enterprise.java (2)
M abiquo/src/main/java/org/jclouds/abiquo/domain/enterprise/Limits.java (30)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/17.patch
https://github.com/jclouds/jclouds-labs/pull/17.diff
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #225](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/225/) 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-labs/pull/17#issuecomment-21786168
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -57,8 +64,8 @@ public void update() {
>
> // Builder
>
> - public static Builder builder(final ApiContext<AbiquoApi> context) {
> - return new Builder(context);
> + public static Builder builder(final ApiContext<AbiquoApi> context, Datacenter datacenter) {
There is no backwards compatible version, but I don't think it should be a problem. This builder will rarely be used directly, since the functionality is usually invoked from the Enterprise.allowDatacenter method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17/files#r5525192
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by carlosgarciaibanez <no...@github.com>.
The suggested changes have been included. Thank you all for the tips and the support!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17#issuecomment-21844041
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Ignasi Barrera <no...@github.com>.
Just a quick note on backwards compatibility (because the issue mentions Abiquo 2.4), this change should not affect previous Abiquo versions, since older versions of the API will just ignore the new link.
This PR looks good to me, once the `null` check has been added to the builder. Thanks @carlosgarciaibanez!
And a tip: Next time (there is no need to change that now), it is better to create a branch in your fork for the changes, and don't work directly on `master`. You can send the PR from your topic branch.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17#issuecomment-21807958
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Andrew Phillips <no...@github.com>.
> @@ -57,8 +64,8 @@ public void update() {
>
> // Builder
>
> - public static Builder builder(final ApiContext<AbiquoApi> context) {
> - return new Builder(context);
> + public static Builder builder(final ApiContext<AbiquoApi> context, Datacenter datacenter) {
I assume there is no way to provide a backwards-compatible version of this call? Then again, this is `master`, so probably no need...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17/files#r5493482
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Andrew Phillips <no...@github.com>.
> super();
> this.context = context;
> + this.datacenter = datacenter;
Same goes for `context`, incidentally, although that should probably be a separate commit. And the `context` and `datacenter` fields could also be `final`, it seems.
What is the call to `super()` for?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17/files#r5493449
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds-labs #230](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-labs/230/) 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-labs/pull/17#issuecomment-21844077
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Andrew Phillips <no...@github.com>.
> @@ -96,7 +110,8 @@ public Limits build() {
> }
>
> public static Builder fromEnterprise(final Limits in) {
> - return Limits.builder(in.context).ramLimits(in.getRamSoftLimitInMb(), in.getRamHardLimitInMb())
> + return Limits.builder(in.context, in.getDatacenter())
> + .ramLimits(in.getRamSoftLimitInMb(), in.getRamHardLimitInMb())
If this is purely a formatting change, align with the following lines?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17/files#r5493461
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Ignasi Barrera <no...@github.com>.
Closed #17.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Ignasi Barrera <no...@github.com>.
> super();
> this.context = context;
> + this.datacenter = datacenter;
Since this field is mandatory, could you add a `checkNotNull` check for it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17/files#r5482503
Re: [jclouds-labs] JCLOUDS-208: Changes for the Abiquo 2.4 procedure
to limit enterprises to datacenters (#17)
Posted by Ignasi Barrera <no...@github.com>.
Squashed the commits and merged to master and 1.6.x.
Thanks @carlosgarciaibanez!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/17#issuecomment-21924976