rackspace / gophercloud

A Go SDK for OpenStack. IN FEATURE FREEZE. See Issue #592
http://gophercloud.io
Other
456 stars 181 forks source link

[rfr] Add OpenStack Image Service v2 (Glance) support #565

Closed sameo closed 8 years ago

sameo commented 8 years ago

This is a revamp of PR #458. I tried to address all the review comments, fixed the build and unit tests and rebased the whole series on top of master/HEAD.

The original PR said:

According to http://developer.openstack.org/api-ref-image-v2.html the following features were implemented

Create image
Update image
Delete image
List images
Upload binary image data
Download binary image data
Create image member
List image members
Show image member details
Delete image member
Update image member

Pull request #443 was used as base.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.6%) to 79.605% when pulling ccfa5ea6d30b9390faa4678fcac833f1307e0e9a on sameo:master into c54bbac81d19eb4df3ad167764dbb6ff2e7194de on rackspace:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.6%) to 79.576% when pulling 98b95d32daa7e11a8fc681693a53b497588c6de8 on sameo:master into c54bbac81d19eb4df3ad167764dbb6ff2e7194de on rackspace:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.6%) to 79.576% when pulling 9e072c5fa4bf445bb14088510905758c5b48ef72 on sameo:master into d62a69f7484d2e2b90c68991ccc91c3309d80b1e on rackspace:master.

jrperritt commented 8 years ago

Great! When this is ready for review, throw a [rfr] tag in front of the title and I'll have a look.

sameo commented 8 years ago

Besides unit testing, we are also using this new feature in Ciao through the ciao-cli tool.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.6%) to 79.972% when pulling 4c05efb6df41da3cba64495b409c673469c60edf on sameo:master into 6fbd243473c9984e40119ce8b96be8bfd1cb75d8 on rackspace:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.6%) to 79.988% when pulling 93ff0ab265a42aec040bf1e269f7034a164cd212 on sameo:master into 6fbd243473c9984e40119ce8b96be8bfd1cb75d8 on rackspace:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.6%) to 79.988% when pulling 7e7d19740ad94a5aed8095c70c17e7fb3c2193b5 on sameo:master into 6fbd243473c9984e40119ce8b96be8bfd1cb75d8 on rackspace:master.

sameo commented 8 years ago

I tried to address all 3 comments from @jrperritt with a new patchet (Also rebased on top of HEAD).

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 80.209% when pulling 5b4f13e6f8afe05019b01c5e910e99aff6e7bc06 on sameo:master into 6fbd243473c9984e40119ce8b96be8bfd1cb75d8 on rackspace:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.4%) to 80.172% when pulling f947641ca18f06446aab929f17dd9a78da06d739 on sameo:master into 6fbd243473c9984e40119ce8b96be8bfd1cb75d8 on rackspace:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.4%) to 80.172% when pulling 75b585dafb48ddae9839c3183e4728226482e3be on sameo:master into 6fbd243473c9984e40119ce8b96be8bfd1cb75d8 on rackspace:master.

sameo commented 8 years ago

@jrperritt Please let me know if there's anything else you'd like to see changed/fixed.

sameo commented 8 years ago

@jrperritt Thanks for the review. New patchset pushed with the following changes:

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.4%) to 80.573% when pulling 2510c2251cbbdd8b716554d770fe6e30945afc7a on sameo:master into d47105ce4ef90cea9a14b85c8dd172b760085828 on rackspace:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.4%) to 80.71% when pulling 362ad10deaa1531b1cd1344df4063d9cee5f87b5 on sameo:master into f28452dea119b3fc68a303e5ac3a4f5fd7e67012 on rackspace:master.

sameo commented 8 years ago

@jrperritt I rebased this PR on top of HEAD and we added a few more OpenStack specified image properties. Please let me know if this needs more work.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.4%) to 80.71% when pulling d71ec9c0ee6f3455e2efbef29641816846c4b775 on sameo:master into f28452dea119b3fc68a303e5ac3a4f5fd7e67012 on rackspace:master.

sameo commented 8 years ago

@jrperritt Should I move this PR to the new github repo ?

jrperritt commented 8 years ago

No, it's fine here. Sorry for the delay; I'll review this again tonight. Once merged, I'll move it over to the new repo.

jrperritt commented 8 years ago

Re-reviewing tonight didn't happen; Migrating a few PRs to the new repo took longer than expected. Shooting for tomorrow (Tuesday).

jrperritt commented 8 years ago

Right. I glanced over it again, and I think it looks good enough to merge by this repo's standards. The new repo is quite stricter on style/naming, but I'll take care of that when I migrate this over there. +2

gberginc commented 8 years ago

@jrperritt Not trying to be pushy, but I am interested if there is a rough time plan for this PR to be merged into the new repository, if at all?

We are working on a fork of the Unik project to add support for OpenStack provider. Since we are moving towards the first PR, I was wondering whether we should use the old one or wait for a bit more to have it in the new repository? Perhaps if you know what would need to be changed, we could help out.

jrperritt commented 8 years ago

I've already started it, but bugs get precedence. I'm currently working on https://github.com/gophercloud/gophercloud/issues/50, then going back to this one. I expect Glance v2 to get merged in gophercloud/gophercloud the next 2 weeks. Please direct future issues/questions to the new repo.