You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by inbar stolberg <no...@github.com> on 2014/10/08 08:21:21 UTC
[jclouds] added tenantId/projectId to volume and snapshot (#562)
added tenant id to volume and project id to snapshot
You can merge this Pull Request by running:
git pull https://github.com/inbarsto/cloudband-jclouds volumeSnapshotTenantProject
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/562
-- Commit Summary --
* added tenantId/projectId to volume and snapshot
-- File Changes --
M apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/Snapshot.java (36)
M apis/openstack-cinder/src/main/java/org/jclouds/openstack/cinder/v1/domain/Volume.java (41)
M apis/openstack-cinder/src/test/java/org/jclouds/openstack/cinder/v1/features/SnapshotApiExpectTest.java (17)
M apis/openstack-cinder/src/test/java/org/jclouds/openstack/cinder/v1/features/VolumeAndSnapshotApiLiveTest.java (5)
M apis/openstack-cinder/src/test/java/org/jclouds/openstack/cinder/v1/features/VolumeApiExpectTest.java (18)
M apis/openstack-cinder/src/test/resources/volume_list_details.json (1)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/562.patch
https://github.com/jclouds/jclouds/pull/562.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
:/ not sure why cloudbees fails ran locally ran fine
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60509558
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1764](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1764/) 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/pull/562#issuecomment-58547113
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1259](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1259/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-58319845
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
> @@ -219,6 +219,22 @@ protected Volume testVolume() {
> .build();
> }
>
> + protected Volume testVolumeDetailed() {
> + return Volume.builder()
> + .id("60761c60-0f56-4499-b522-ff13e120af10")
> + .size(1)
> + .name("test")
> + .zone("nova")
> + .status(Volume.Status.IN_USE)
> + .volumeType("None")
> + .description("This is a test volume")
> + .attachments(ImmutableSet.of(testAttachment()))
> + .created(dateService.iso8601DateParse("2012-10-29T20:53:28.000000"))
> + .tenantId("0ad7eca25ff847b2947a7865b82b851c")
> + .build();
> + }
> +
removed
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18657547
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Jeremy Daggett <no...@github.com>.
>
> @ConstructorProperties({
> - "id", "volume_id", "status", "size", "created_at", "display_name", "display_description"
> + "id", "volume_id", "status", "size", "created_at", "display_name", "display_description", "os-extended-snapshot-attributes:project_id"
Hmm, I am not sure that we want to keep adding to the constructors for extension types. Is actually a `progress` field as well from the latest on [master](https://github.com/openstack/cinder/blob/master/cinder/api/contrib/extended_snapshot_attributes.py#L44).
Maybe extend Snapshot? `public class ExtendedSnapshot extends Snapshot`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18594151
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
The change works as is. I believe I do. I would rather this to be merged as is because this or is open for a long time now and I have no use for the second field. I have no problem adding the other field in a second PR if we agree it is better with the second field as well....
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-62408163
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
removed abstract builders for volume and snapshot + added snapshot extended attributes
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59640731
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #254](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/254/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60508588
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Adrian Cole <no...@github.com>.
I agree. Fields are easy to add hard to remove. Add what you need!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-62410166
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #210](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/210/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59641029
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
@inbarsto I was just about to merge this when I saw @jdaggett's comment that "I just noticed that this class is missing a field".
Do you know which field Jeremy might be talking about? I'm assuming your change works as-is?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-62404044
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1812](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1812/) 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/pull/562#issuecomment-59688409
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
>
> @ConstructorProperties({
> - "id", "volume_id", "status", "size", "created_at", "display_name", "display_description"
> + "id", "volume_id", "status", "size", "created_at", "display_name", "display_description", "os-extended-snapshot-attributes:project_id"
@jdaggett Ping about @inbarsto's response..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18748203
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Adrian Cole <no...@github.com>.
> @@ -208,9 +221,17 @@ public String getDescription() {
> return this.description;
> }
>
> + /**
> + * @return the project id of this snapshot - as displayed in the openstack console
Nit. add the extension required that is needed for this to ever be present. Ex. Only present when {@code os-vol-tenant-attr} extension is. see ExtensionApi
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18588122
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> @Override
> protected void configure() {
> bind(DateAdapter.class).to(GsonModule.Iso8601DateAdapter.class);
> }
> +
> + @Singleton
> + public static class SnapshotAddapter implements JsonDeserializer<Snapshot> {
[minor] Probably "Adapter" (typo) - again, we can fix this during the merge.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063490
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
Sure does. Thanks Andrew :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-61955782
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.domain;
> +
> +import com.google.common.base.Objects;
> +import org.jclouds.javax.annotation.Nullable;
> +
> +import javax.inject.Named;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Additional attributes delivered by Extended Snapshot Attributes extensions
@everett-toews @zack-shoylev @jdaggett Ping?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19323436
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.domain;
> +
> +import com.google.common.base.Objects;
> +import org.jclouds.javax.annotation.Nullable;
> +
> +import javax.inject.Named;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Additional attributes delivered by Extended Snapshot Attributes extensions
Is the part of the [Volume Extensions](http://docs.openstack.org/api/openstack-compute/2/content/ext-os-volume.html), or a different extension?
/cc @everett-toews @zack-shoylev @jdaggett
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19087735
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
> @@ -117,6 +118,7 @@ public void testListVolumesInDetail() {
> assertEquals(details.getName(), vol.getName());
> assertEquals(details.getDescription(), vol.getDescription());
> assertEquals(details.getCreated(), vol.getCreated());
> + assertEquals(details.getTenantId(), vol.getTenantId());
> if (Objects.equal(details.getId(), testVolume.getId())) {
> foundIt = true;
done
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19068627
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> not sure why cloudbees fails ran locally ran fine
Does it complain if you run `mvn clean verify` or `mvn checkstyle:check` locally? It _should_, because that's what [DEV@cloud doesn't like](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1343/org.apache.jclouds.api$openstack-cinder/violations/).
No need for you to bother with fixing these, though - we can clean them up during the merge.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60531916
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Jeremy Daggett <no...@github.com>.
>
> @ConstructorProperties({
> - "id", "volume_id", "status", "size", "created_at", "display_name", "display_description"
> + "id", "volume_id", "status", "size", "created_at", "display_name", "display_description", "os-extended-snapshot-attributes:project_id"
@inbarsto Go ahead and leave it for now, thx.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18781582
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #179](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/179/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-58546810
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> Would u like me to create a new PR for it?
No need, we can happily do that for you. I'll create one in a sec...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-62596665
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> @@ -142,11 +151,10 @@ protected ConcreteBuilder self() {
> private final String name;
> @Named("display_description")
> private final String description;
> + private final Optional<SnapshotExtendedAttributes> extendedAttributes;
[minor] Does this also need a `@Named`? We can also clean this up during the merge, by the way...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063477
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1303](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1303/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59686824
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
>
> @ConstructorProperties({
> - "id", "volume_id", "status", "size", "created_at", "display_name", "display_description"
> + "id", "volume_id", "status", "size", "created_at", "display_name", "display_description", "os-extended-snapshot-attributes:project_id"
hmm, I just found some thing in the code i think we will all agree on,
pushing soon
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18827309
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
> @@ -231,11 +237,11 @@ public static Volume forId(String volumeId) {
> @Named("display_description")
> private final String description;
> private final Map<String, String> metadata;
nope the field name is "metadata"...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19068261
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
Thank you. Yeah it need to go to master. Would u like me to create a new PR for it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-62475042
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
i keep running and it doesnt work locally, so i dont know what r the issues, if you can send me the issues i'll fix them...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60754945
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1807](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1807/) 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/pull/562#issuecomment-59641610
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
> @@ -142,11 +151,10 @@ protected ConcreteBuilder self() {
> private final String name;
> @Named("display_description")
> private final String description;
> + private final Optional<SnapshotExtendedAttributes> extendedAttributes;
nope, see:
https://github.com/jclouds/jclouds/blob/1.8.x/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/Server.java#L311
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19068231
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.domain;
> +
> +import com.google.common.base.Objects;
> +import org.jclouds.javax.annotation.Nullable;
> +
> +import javax.inject.Named;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Additional attributes delivered by Extended Snapshot Attributes extensions
sorry but i don't know where to find it :(
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19068393
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Jeremy Daggett <no...@github.com>.
> + return new SnapshotExtendedAttributes(projectId);
> + }
> +
> + public Builder fromExtendedAttributes(SnapshotExtendedAttributes in) {
> + return this
> + .projectId(in.getProjectId());
> + }
> +
> + protected Builder self() {
> + return this;
> + }
> + }
> +
> + @Named("os-extended-snapshot-attributes:project_id")
> + private final String projectId;
> +
@inbarsto There is an additional `os-extended-snapshot-attributes:progress` field defined in the [API]( https://github.com/openstack/cinder/blob/master/cinder/api/contrib/extended_snapshot_attributes.py#L75) that will need to be added.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19347847
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
fixing the snapshot extension and the builders... will push soon
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59054866
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
is this good to go? (at least on master until 1.8 is finished?)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60043895
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> +
> + snapshotBase = apply((SnapshotInteranl) context.deserialize(jsonElement, SnapshotInteranl.class));
> +
> + Snapshot.Builder result = Snapshot.builder().fromSnapshot(snapshotBase);
> + SnapshotExtendedAttributes extendedAttributes = context.deserialize(jsonElement, SnapshotExtendedAttributes.class);
> + if (!Objects.equal(extendedAttributes, SnapshotExtendedAttributes.builder().build())) {
> + result.extendedAttributes(extendedAttributes);
> + }
> + return result.build();
> + }
> +
> + public Snapshot apply(Snapshot in) {
> + return in.toBuilder().build();
> + }
> +
> + private static class SnapshotInteranl extends Snapshot {
[minor] This is probably meant to be "Internal" (typo) - we can clean this up during the merge.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063483
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1858](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1858/) 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/pull/562#issuecomment-60509129
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
Thanks, @inbarsto! Mainly minor comments - only question to address for me would be whether some of the commented-out code is intentional.
1.8.x should be unblocked soon, but I guess you'll also want this to be applied to master?
Also (the usual question, by now ;-)): could you give some background to this? Are those two fields required for a specific use case or..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59666102
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by cloudband <no...@github.com>.
> @@ -208,9 +221,17 @@ public String getDescription() {
> return this.description;
> }
>
> + /**
> + * @return the project id of this snapshot - as displayed in the openstack console
from what i see it isn't there :/ inside these apis so i don't know :/
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19091446
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Jeremy Daggett <no...@github.com>.
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.domain;
> +
> +import com.google.common.base.Objects;
> +import org.jclouds.javax.annotation.Nullable;
> +
> +import javax.inject.Named;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Additional attributes delivered by Extended Snapshot Attributes extensions
@demobox Hi! No, this is actually the OpenStack Cinder Extended Volume Attriibutes [extension](https://github.com/openstack/cinder/blob/master/cinder/api/contrib/extended_snapshot_attributes.py). I just noticed that this class is missing a field, so I plan to do full review of this PR when I get some time today.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19347482
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
@inbarsto I'm here ;-) If we don't hear today or tomorrow, I will merge this, I think - we can always revert/update if necessary. Does that work for you?
/cc @jdaggett @zack-shoylev @everett-toews
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-61954440
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -231,11 +237,11 @@ public static Volume forId(String volumeId) {
> @Named("display_description")
> private final String description;
> private final Map<String, String> metadata;
It is unnecessary. If the field name is the same, then it's fine as-is.
The obscure reflection thing happens when there is no `@ConstructorProperties` annotation. The class does not have a default constructor, but a constructor that accepts "some" fields, and there is no hint about which parameter corresponds to each field in the JSON object. In that case, if the object is immutable (fields are final and there are no setters), Gson uses the `sun.misc.Unsafe` class to directly populate the values to the fields, based on their name.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19086373
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Jeremy Daggett <no...@github.com>.
> @@ -219,6 +219,22 @@ protected Volume testVolume() {
> .build();
> }
>
> + protected Volume testVolumeDetailed() {
> + return Volume.builder()
> + .id("60761c60-0f56-4499-b522-ff13e120af10")
> + .size(1)
> + .name("test")
> + .zone("nova")
> + .status(Volume.Status.IN_USE)
> + .volumeType("None")
> + .description("This is a test volume")
> + .attachments(ImmutableSet.of(testAttachment()))
> + .created(dateService.iso8601DateParse("2012-10-29T20:53:28.000000"))
> + .tenantId("0ad7eca25ff847b2947a7865b82b851c")
> + .build();
> + }
> +
remove space
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18594704
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> @@ -231,11 +237,11 @@ public static Volume forId(String volumeId) {
> @Named("display_description")
> private final String description;
> private final Map<String, String> metadata;
[minor] This is not part of your PR, I know, but does this also need a `@Named`?
/cc @nacx
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063506
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
Merged (finally!) to [1.8.x](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=dedeb63b0fc280e0653e63ab678505d45feea51c). Please check out the commit.
Does this also need to go to master?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-62424564
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> foundIt = true;
> - }
> + //}
Is this intentionally commented-out? If so, remove?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063521
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
ping
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-61937124
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
added progress and fixed toString in snapshotExtendedAttributes
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60508518
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
@demobox ping?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-62311419
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
yeah definitely to master and 1.8.x.
the use case is for us to run on a multi tenancy openstack we need to know what resource belongs to which object.
(there is a flag that you can use that brings in a list all the resources of a certain type for all the tenants)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59685608
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1268](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1268/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-58548887
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> @@ -208,9 +221,17 @@ public String getDescription() {
> return this.description;
> }
>
> + /**
> + * @return the project id of this snapshot - as displayed in the openstack console
@inbarsto Is this maybe the ["Volume extension"](http://docs.openstack.org/api/openstack-compute/2/content/ext-os-volume.html)? I.e. "Only present when {@code os-volumes, os-snapshots} extension is. See ExtensionApi"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19087627
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1343](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1343/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60508699
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #1299](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/1299/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59641114
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> @@ -231,11 +237,11 @@ public static Volume forId(String volumeId) {
> @Named("display_description")
> private final String description;
> private final Map<String, String> metadata;
Ah, OK. Thanks for explaining, @nacx!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19323435
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
>
> @ConstructorProperties({
> - "id", "volume_id", "status", "size", "created_at", "display_name", "display_description"
> + "id", "volume_id", "status", "size", "created_at", "display_name", "display_description", "os-extended-snapshot-attributes:project_id"
wont do it nullable be simpler and will present a cleaner code?
what will happen when we will have multiple extension?
if you still want to go with public class ExtendedSnapshot extends Snapshot let me know i'll do it...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r18657456
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #170](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/170/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-58318553
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #1753](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/1753/) 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/pull/562#issuecomment-58337535
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> }
> - assertTrue(foundIt, "Failed to find the snapshot we previously created in listSnapshots() response");
> + // assertTrue(foundIt, "Failed to find the snapshot we previously created in listSnapshots() response");
Is this intentionally commented-out?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063524
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests-java-6 #214](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests-java-6/214/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59686478
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Jeremy Daggett <no...@github.com>.
> + @Override
> + public int hashCode() {
> + return Objects.hashCode(projectId);
> + }
> +
> + @Override
> + public boolean equals(Object obj) {
> + if (this == obj) return true;
> + if (obj == null || getClass() != obj.getClass()) return false;
> + SnapshotExtendedAttributes that = SnapshotExtendedAttributes.class.cast(obj);
> + return Objects.equal(this.projectId, that.projectId);
> + }
> +
> + protected Objects.ToStringHelper string() {
> + return Objects.toStringHelper(this)
> + .add("extendedAttributes", projectId);
This should match the name of the field in the class: `.add("projectId", projectId);` This will also apply to the `progress` field (when added).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19347993
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> @@ -231,11 +237,11 @@ public static Volume forId(String volumeId) {
> @Named("display_description")
> private final String description;
> private final Map<String, String> metadata;
> nope the field name is "metadata"...
@nacx Just to clarify my understanding: is it better to have `@Named` on attributes where the name is the same as the annotation, or is that unnecessary? I recall some discussion about this making a difference in terms or forcing the code to do reflection, or something like that...but would rather ask to clarify ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19085723
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> @@ -117,6 +118,7 @@ public void testListVolumesInDetail() {
> assertEquals(details.getName(), vol.getName());
> assertEquals(details.getDescription(), vol.getDescription());
> assertEquals(details.getCreated(), vol.getCreated());
> + assertEquals(details.getTenantId(), vol.getTenantId());
> if (Objects.equal(details.getId(), testVolume.getId())) {
> foundIt = true;
Does this also need a `break`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063519
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> if you can send me the issues i'll fix them...
You should be able to click on the DEV@cloud links - those builds and the violations results are public. But don't worry about fixing those now, we can take care of that during the merge.
Only waiting on Jeremy now, since he mentioned he was going to look at this again.
@jdaggett: ping..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-60863212
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by inbar stolberg <no...@github.com>.
so... good to go?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562#issuecomment-59001756
Re: [jclouds] added tenantId/projectId to volume and snapshot (#562)
Posted by Andrew Phillips <no...@github.com>.
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +package org.jclouds.openstack.cinder.v1.domain;
> +
> +import com.google.common.base.Objects;
> +import org.jclouds.javax.annotation.Nullable;
> +
> +import javax.inject.Named;
> +import java.beans.ConstructorProperties;
> +
> +/**
> + * Additional attributes delivered by Extended Snapshot Attributes extensions
Following on from Adrian's comment above: does this extension have a jclouds equivalent? If so, link to that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/562/files#r19063498