I'm fighing lbox. When I updated this it put it over at a new url.

https://codereview.appspot.com/52440043


https://codereview.appspot.com/51470044/diff/1/README.md
File README.md (right):

https://codereview.appspot.com/51470044/diff/1/README.md#newcode76
README.md:76: core) or "admin" (for pyjuju). The password is the same as
your Juju
On 2014/01/14 17:26:36, frankban wrote:
> We can remove the pyjuju reference here.

Thanks, removed.

https://codereview.appspot.com/51470044/diff/1/README.md#newcode105
README.md:105: For both Juju Core and PyJuju, you must simply do the
following steps.  Note
On 2014/01/14 17:26:36, frankban wrote:
> And here too...

Removed

https://codereview.appspot.com/51470044/diff/1/README.md#newcode162
README.md:162: #### pyjuju ####
On 2014/01/14 17:26:36, frankban wrote:
> We can get rid of this paragraph and of all the juju-jitsu stuff \o/


Removed

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode87
hooks/utils.py:87: Serializer, su,
On 2014/01/14 17:26:36, frankban wrote:
> Please keep "su" in its own line.

Mistake, thanks for the catch.

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode148
hooks/utils.py:148: return api_addresses.split()[0]
On 2014/01/14 17:26:36, frankban wrote:
> So you decided to stop supporting also old releases of juju-core.
> I am not sure about this: at least we should document it more
explicitly.

Can you talk about this more? My understanding is that we got rid of the
agent bit and that was the conf file we've removed as part of that. Is
the machiner agent file something different?

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode320
hooks/utils.py:320: password = password if password else None
On 2014/01/14 17:26:36, frankban wrote:
> Maybe rewrite those three lines like the following?
> if not password:
>      password = 'admin' if sandbox else None

Tweaked.

https://codereview.appspot.com/51470044/diff/1/hooks/utils.py#newcode362
hooks/utils.py:362: # In PyJuju environments, use the same certificate
for both HTTPS and
On 2014/01/14 17:26:36, frankban wrote:
> Please remove this reference to PyJuju.

Removed

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test
File tests/20-functional.test (right):

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newcode45
tests/20-functional.test:45: STAGING_SERVICES = ('haproxy', 'mediawiki',
'memcached', 'mysql', 'wordpress')
On 2014/01/14 17:26:36, frankban wrote:
> STAGING_SERVICES seems to be no longer required.

Wow, how did lint not catch that? Thanks.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newcode46
tests/20-functional.test:46: juju_version().major == 0
On 2014/01/14 17:26:36, frankban wrote:
> Can't we remove this line?
> Moreover, can't we remove the juju_version function entirely?

Oh hmm, I didn't notice it was only used for pyjuju vs juju-core
distinction. Removing.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newcode78
tests/20-functional.test:78: if options is None:
On 2014/01/14 17:26:36, frankban wrote:
> This seems not right. The cleanup_services list was only required in
pyJuju, and
> now we are using it in juju-core. IIRC this whole function is a
wrapper for
> juju_deploy, and was required to workaround the pyJuju bug descibed in
the XXX
> above. AFAICT now it is no longer required for this function to return
the list
> of services to be stopped.

Sorry, I meant to ask about this one. The bug was noted as wrapping and
adding a cleanup. It does set self.service_name and only returns the
unit_info so I wasn't sure if it made sense to keep it as it kept a
specific api.

It sounds like no, all calls to self.deploy_gui should just be updated
to juju_deploy_gui. I'll update.

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newcode180
tests/20-functional.test:180: # XXX 2013-11-27 frankban bug=872264:
On 2014/01/14 17:26:36, frankban wrote:
> Please remove these comments. The comment above suggests we could also
remove
> this method all together.

Removed

https://codereview.appspot.com/51470044/diff/1/tests/20-functional.test#newcode262
tests/20-functional.test:262: # XXX 2013-11-27 frankban bug=872264:
On 2014/01/14 17:26:36, frankban wrote:
> These comments must go away.

gone

https://codereview.appspot.com/51470044/