Rackspace Server/Image metadata operations #1452

Merged
merged 11 commits into from Jan 9, 2013

Conversation

Projects
None yet
4 participants
Owner

krames commented Jan 7, 2013

Updated Fog::Compute::RackspaceV2::Server and Fog::Compute::RackspaceV2::Image to support metadata operations. The implementation was inspired by the OpenStack provider.

Owner

geemus commented Jan 7, 2013

@krames - seems good, thanks!

@bradgignac @brianhartsock - could one of you review? Thanks!

+
+ tests("#get('my_key')").succeeds do
+ pending if Fog.mocking? && !mocks_implemented
+ @image.metadata.get('my_key')
@bradgignac

bradgignac Jan 7, 2013

Member

Should this test be asserting on the data returned?

@krames

krames Jan 7, 2013

Owner

I was wondering that as well. I dug though some of the other tests out there and it didn't seem like the model tests did not assert return values. I figured that was a matter of convention. I can update these tests to test the return if you think it's necessary.

+ @image.wait_for(timeout = 1500) { ready? }
+ tests("#all").succeeds do
+ pending if Fog.mocking? && !mocks_implemented
+ @image.metadata.all
@bradgignac

bradgignac Jan 7, 2013

Member

Should this test be asserting on the data returned?

Member

bradgignac commented Jan 7, 2013

Aside from one question about test, this looks good to me. I'd like to hear @brianhartsock's feedback though.

Member

bradgignac commented Jan 7, 2013

Also, it might be worth rebasing and adding [rackspace|compute] to the beginning of commit messages. I believe @geemus uses this metadata when generating the changelog for a release.

Owner

geemus commented Jan 7, 2013

@bradgignac - yep, the prefix on messages wouldn't be the end of the world, but it helps group the changelog by provider instead of putting things in the "misc" catch all/default.

added better acceptance criteria to server/image metadata tests; adde…
…d metadata to ignored_attributes to address bug
Member

brianhartsock commented Jan 8, 2013

LGTM, just looks like conflicts with master need to be resolved.

Owner

krames commented Jan 8, 2013

I will take a look at this first thing this morning.

Owner

krames commented Jan 8, 2013

@bradgignac I updated it with the lastest master and it passes all of the tests. Let me know if you need me to look at anything else.

Thanks!

bradgignac added a commit that referenced this pull request Jan 9, 2013

Merge pull request #1452 from rackspace/server_metadata
Rackspace Server/Image metadata operations

@bradgignac bradgignac merged commit a8d159f into fog:master Jan 9, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment