Cleanup round 1 of azurecompute: Output value types. #92

codefromthecrypt · 2014-10-15T15:14:57Z

Main changes are consolidating and cleaning output value objects, like Deployment. Some time in the future, we can remove a lot more code and also the guava dependency from these classes via @AutoValue

codefromthecrypt · 2014-10-15T15:15:04Z

cc @abayer

codefromthecrypt · 2014-10-16T17:52:47Z

For the passer-by, I'm doing slow-burn cleanup, and also playing with ideas wrt where we actually need builders or defensive enums. Current thinking is that we don't unless it is an in-out or input value object. Once I'm done cleanup here, I can present a coherent idea back to the group. That may not happen until a week from now.

cloudbees-pull-request-builder · 2014-10-17T05:18:58Z

jclouds-labs-pull-requests #320 SUCCESS
This pull request looks good

buildhive · 2014-10-17T05:30:37Z

jclouds » jclouds-labs #1701 SUCCESS
This pull request looks good
(what's this?)

codefromthecrypt · 2014-10-18T14:07:36Z

Live tests still pass. Can't migrate to google auto value yet as we use a custom @Nullable annotation. The fix for that was merged in Feb, but there's not been a new release, yet. Pinged them.

Also note. I'm only about 1/2 done cleaning objects anyway.

cloudbees-pull-request-builder · 2014-10-18T14:13:35Z

jclouds-labs-pull-requests #322 SUCCESS
This pull request looks good

buildhive · 2014-10-18T14:49:55Z

jclouds » jclouds-labs #1710 SUCCESS
This pull request looks good
(what's this?)

demobox · 2014-10-18T18:49:50Z

azurecompute/README.md

@@ -20,4 +20,4 @@ Once you do this, you will set the following to run the live tests.
 mvn -Plive -Dtest.jclouds.azurecompute.subscription-id=12345678-abcd-dcba-abdc-ba0987654321
 -Dtest.azurecompute.credential=P12_EXPORT_PASSWORD
 -Dtest.azurecompute.identity=$HOME/.jclouds/azure.p12


[minor] Should this also be .pem? If so, happy to fix this during merge...

nope! actually this should be p12 for now anyway. I haven't refactored the authentication bit, yet, and was surprised myself at this!

demobox · 2014-10-18T19:06:38Z

Thanks for another big cleanup, @adriancole! +1 - looks good to me.

A couple of minor questions, but they can be dealt with, if at all, in later stages of the cleanup.

codefromthecrypt · 2014-10-19T12:41:55Z

thanks for the review, @demobox! I'll ping you when done the last cleanup round.

codefromthecrypt · 2014-10-19T14:38:07Z

added disk, the last biggie. I still have some consistency cleanups to do, and the input objects.

codefromthecrypt · 2014-10-19T14:41:12Z

ps so far I'm absolutely convinced we should end the practice of builders on output-only value types.

cloudbees-pull-request-builder · 2014-10-19T14:45:46Z

jclouds-labs-pull-requests #323 SUCCESS
This pull request looks good

buildhive · 2014-10-19T14:56:26Z

jclouds » jclouds-labs #1715 SUCCESS
This pull request looks good
(what's this?)

demobox · 2014-10-19T21:26:19Z

we should end the practice of builders on output-only value types.

Do you recall where this came from in the first place..?

demobox · 2014-10-19T21:28:48Z

azurecompute/src/main/java/org/jclouds/azurecompute/xml/DeploymentHandler.java

+
+   @VisibleForTesting static InstanceStatus parseInstanceStatus(String instanceStatus) {
+      try {
+         // Azure isn't exactly upper-camel, as some states end in VM, not Vm.


Ahhh...this is what the V_M was all about!

codefromthecrypt · 2014-10-20T14:16:51Z

@demobox wrt builder for output-only.. I think it was probably my fault. I'm sure I suggested that we use builders, but didn't think through that they are only relevant for input objects.

codefromthecrypt · 2014-10-20T14:20:08Z

OK. all output objects are clean. I'd expect another 1K or so less lines when auto-value cuts another release.

Note: I decided to not use MoreObjects.toStringHelper for a few reasons.

Objects.toStringHelper will be around until June 2016, so a little early to worry about, esp in labs
Using MoreObjects.toStringHelper makes backporting a royal pita
I won't need any toStringHelper once auto-value is hooked up, thus avoiding the entire thrash problem for this class of guava usage.

@demobox ready for your review! After this, I'll work on the input objects, then update the api, then move on to compute service.

cloudbees-pull-request-builder · 2014-10-20T14:22:41Z

jclouds-labs-pull-requests #324 SUCCESS
This pull request looks good

buildhive · 2014-10-20T14:32:23Z

jclouds » jclouds-labs #1718 SUCCESS
This pull request looks good
(what's this?)

demobox · 2014-10-20T17:08:34Z

azurecompute/src/main/java/org/jclouds/azurecompute/domain/Disk.java

+      this.description = description;
+      this.os = os;
+      this.mediaLink = mediaLink;
+      this.logicalSizeInGB = checkNotNull(logicalSizeInGB, "logicalSizeInGB of %s", name);


This is listed above as @Nullable?

nullable was wrong. fixed

demobox · 2014-10-20T17:20:33Z

Only one comment that might need to be looked at now (a field marked @Nullable but with a checkNotNull) - the rest can all be ignored and/or handled later, if desired.

+1 - looks good to me! Thanks for more cleanup, @adriancole!

cloudbees-pull-request-builder · 2014-10-20T17:28:40Z

jclouds-labs-pull-requests #325 SUCCESS
This pull request looks good

buildhive · 2014-10-20T17:38:47Z

jclouds » jclouds-labs #1719 SUCCESS
This pull request looks good
(what's this?)

ccustine · 2014-10-20T17:58:31Z

+1 btw, what are we waiting on before using autovalue? A specific issue or feature?

codefromthecrypt · 2014-10-20T19:50:40Z

@ccustine there's a snapshot published to sonatype, but no stable version. Since we use our own Nullable annotation, auto-value 1.0-rc1 can't work, but anything after Feb will :)

codefromthecrypt · 2014-10-20T19:59:06Z

merged into master. will do 1.8.x once 1.8.1 is cut.

Fix osgi export.

99762af

codefromthecrypt force-pushed the master branch from bcec4f6 to c005ff7 Compare October 18, 2014 14:01

Adrian Cole added 2 commits October 18, 2014 10:04

Fixed README bug.

1952095

Removed unused JAXB annotations.

d8280fd

demobox reviewed Oct 18, 2014
View reviewed changes

demobox reviewed Oct 19, 2014
View reviewed changes

Adrian Cole and others added 2 commits October 20, 2014 08:41

Cleanup output-only Deployment object.

b5e2b01

Cleanup output-only HostedService object.

8720276

codefromthecrypt changed the title ~~Cleanup round 1 of azurecompute Deployment class and imports.~~ Cleanup round 1 of azurecompute: Output value types. Oct 20, 2014

demobox reviewed Oct 20, 2014
View reviewed changes

Adrian Cole added 5 commits October 20, 2014 13:20

Cleanup output-only Image object.

1bc0a66

Cleanup output-only Disk object.

118fc24

Cleanup output-only Error object.

fbf8522

Cleanup output-only Location object.

5c6ec27

Cleanup output-only Operation object.

b6ba8e0

codefromthecrypt closed this Oct 20, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup round 1 of azurecompute: Output value types. #92

Cleanup round 1 of azurecompute: Output value types. #92

codefromthecrypt commented Oct 15, 2014

codefromthecrypt commented Oct 15, 2014

codefromthecrypt commented Oct 16, 2014

cloudbees-pull-request-builder commented Oct 17, 2014

buildhive commented Oct 17, 2014

codefromthecrypt commented Oct 18, 2014

cloudbees-pull-request-builder commented Oct 18, 2014

buildhive commented Oct 18, 2014

demobox Oct 18, 2014

codefromthecrypt Oct 19, 2014

demobox commented Oct 18, 2014

codefromthecrypt commented Oct 19, 2014

codefromthecrypt commented Oct 19, 2014

codefromthecrypt commented Oct 19, 2014

cloudbees-pull-request-builder commented Oct 19, 2014

buildhive commented Oct 19, 2014

demobox commented Oct 19, 2014

demobox Oct 19, 2014

codefromthecrypt commented Oct 20, 2014

codefromthecrypt commented Oct 20, 2014

cloudbees-pull-request-builder commented Oct 20, 2014

buildhive commented Oct 20, 2014

demobox Oct 20, 2014

codefromthecrypt Oct 20, 2014

demobox commented Oct 20, 2014

cloudbees-pull-request-builder commented Oct 20, 2014

buildhive commented Oct 20, 2014

ccustine commented Oct 20, 2014

codefromthecrypt commented Oct 20, 2014

codefromthecrypt commented Oct 20, 2014

Cleanup round 1 of azurecompute: Output value types. #92

Cleanup round 1 of azurecompute: Output value types. #92

Conversation

codefromthecrypt commented Oct 15, 2014

codefromthecrypt commented Oct 15, 2014

codefromthecrypt commented Oct 16, 2014

cloudbees-pull-request-builder commented Oct 17, 2014

buildhive commented Oct 17, 2014

codefromthecrypt commented Oct 18, 2014

cloudbees-pull-request-builder commented Oct 18, 2014

buildhive commented Oct 18, 2014

demobox Oct 18, 2014

Choose a reason for hiding this comment

codefromthecrypt Oct 19, 2014

Choose a reason for hiding this comment

demobox commented Oct 18, 2014

codefromthecrypt commented Oct 19, 2014

codefromthecrypt commented Oct 19, 2014

codefromthecrypt commented Oct 19, 2014

cloudbees-pull-request-builder commented Oct 19, 2014

buildhive commented Oct 19, 2014

demobox commented Oct 19, 2014

demobox Oct 19, 2014

Choose a reason for hiding this comment

codefromthecrypt commented Oct 20, 2014

codefromthecrypt commented Oct 20, 2014

cloudbees-pull-request-builder commented Oct 20, 2014

buildhive commented Oct 20, 2014

demobox Oct 20, 2014

Choose a reason for hiding this comment

codefromthecrypt Oct 20, 2014

Choose a reason for hiding this comment

demobox commented Oct 20, 2014

cloudbees-pull-request-builder commented Oct 20, 2014

buildhive commented Oct 20, 2014

ccustine commented Oct 20, 2014

codefromthecrypt commented Oct 20, 2014

codefromthecrypt commented Oct 20, 2014