JCLOUDS-756 - Add support for tags in CloudStack #578
Conversation
|
jclouds-pull-requests-java-6 #216 SUCCESS |
|
jclouds-pull-requests #1305 SUCCESS |
|
jclouds » jclouds #1815 SUCCESS |
| "domainid": 1, | ||
| "key": "some-tag", | ||
| "resourceid": 2, |
There was a problem hiding this comment.
interesting.. so these actually come through as regular json values as opposed to stringly ones. Is it ever the case that a tag value is a hash?
There was a problem hiding this comment.
In the sample data, I only used one tag, but it's a list of hashes.
|
Epic. So, I'm expecting that there will be some live test impact, I guess (plus the mock/expect you mentioned). I'm also assuming you are following convention of existing codebase to avoid confusing things by porting only this to whatever latest style is. I must admit that the tags concept is a bit confusing, particularly the array of things bit vs the map thing. I'd like to see an in-out live test, at least on one resource that explains the relationship between the tags on options and the Tag array thing. Otherwise, just explaining could help. I am probably fishing for whether it is possible to make tags appear less weird, and hide its weirdness inside somehow. Failing that possibility, the general approach looks sound. Thanks for the help! |
|
The tags aren't really confusing - they're just overly verbose, as it were. And yeah, as a copy-paste-coder, I always just use the style in place in the existing code in the API rather than diverging - maybe not ideal, but makes it much easier to read and maintain, IMO. |
|
Heh I would clarify that it is read and maintainence neutral. There's Anyways. Thanks and carry-on! |
|
Ah, but it is easier to maintain an API when the entire API is doing the same thing, even if that thing is a bit wonky. =) |
|
It also has a side benefit of making adrian's hands shake just about enough |
|
..that too, yes. |
|
jclouds-pull-requests-java-6 #218 SUCCESS |
|
jclouds-pull-requests #1307 SUCCESS |
|
jclouds » jclouds #1819 SUCCESS |
|
jclouds-pull-requests-java-6 #219 SUCCESS |
|
jclouds-pull-requests #1308 SUCCESS |
|
jclouds-pull-requests-java-6 #220 SUCCESS |
|
Ok, live tests added and they pass - I think that suffices. Squashing now. |
|
jclouds » jclouds #1820 SUCCESS |
|
jclouds-pull-requests-java-6 #221 SUCCESS |
|
LGTM (gripes noted) Thanks, Andrew! |
|
jclouds-pull-requests #1310 SUCCESS |
|
jclouds-pull-requests #1309 SUCCESS |
|
jclouds » jclouds #1821 SUCCESS |
|
jclouds » jclouds #1822 SUCCESS |
|
jclouds-pull-requests-java-6 #222 SUCCESS |
|
jclouds-pull-requests #1311 SUCCESS |
|
jclouds-pull-requests-java-6 #223 SUCCESS |
|
Ok, added the tags/userMetadata to NodeMetadata, ComputeService-driven node creation, etc. I'm calling this done with another PR to come later on general live test cleanup/fixes. |
|
jclouds-pull-requests-java-6 #224 SUCCESS |
|
jclouds-pull-requests #1312 SUCCESS |
|
jclouds-pull-requests #1313 SUCCESS |
|
jclouds » jclouds #1824 SUCCESS |
|
jclouds » jclouds #1825 SUCCESS |
|
jclouds-pull-requests-java-6 #227 SUCCESS |
|
jclouds-pull-requests #1316 SUCCESS |
|
jclouds » jclouds #1828 UNSTABLE |
Note - this is just a preliminary PR, not ready for merge yet. I still have to add a couple more expect tests, update a couple more, add live tests and update one or two other live tests, but I wanted to throw this up so others could see it.
Note also that yeah, I went a little nutty and reformatted most of the json test files I touched to actually be readable, so the PR may look a bit uglier than it actually is. Hey, going forward, readability's nice, right? =)