Code review comment for lp:~gz/pyjuju/openstack_provider

Revision history for this message
Martin Packman (gz) wrote :

> Just wanted to add some testing results..

Thanks for doing this Clint!

> 1) openstack_s3 works perfectly w/ canonistack. Need to provide a map in
> documentation of Env Var -> environments.yaml values, as they are not clear.

Yes. Is there a good place to document these? With some recent changes, most should be taken from the local environment rather than needing to be copied into the juju yaml file at least.

> 2) I was able to coax the branch with some tweaks to work with hpcloud (which,
> I believe, is a very old OpenStack:
>
> lp:~clint-fewbar/juju/openstack_provider_fixes

This use useful, these are areas I knew the code was weak but hadn't seen it breaking. A couple of queries to help me improve the coverage:

* What exactly went wrong when wrongly setting Accept as json all the time? Swift I expect?
* What is the configured OS_REGION vs the one returned from keystone in the serviceCatalog?

> Note that there's a stupid amount of debug logging that should be removed.

Looking at where you've added that logging is actually useful. Currently the client logs requests and responses, but only after the authentication step to avoid leaking passwords. More targetted debug logging there, given the number of times auth has needed tweaking, would be a good addition.

> Even w/ that branch, one still must manually delete everything on destroy-
> environment, and for some reason machine 0 doesn't get hostname details.

There are some failing tests for the shutdown code, hopefully those cover the issue here but I'll make a note to check when they're passing and if there's more to be done.

« Back to merge proposal