Code review comment for lp://staging/~rharding/charms/precise/juju-gui/remove-pyjuju
Revision history for this message
| Richard Harding (rharding) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
513534c
demo site
(Get the code!)
I'm fighing lbox. When I updated this it put it over at a new url.
https:/
https:/
File README.md (right):
https:/
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:/
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:/
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:/
File hooks/utils.py (right):
https:/
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:/
hooks/utils.py:148: return api_addresses.
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:/
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:/
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:/
File tests/20-
https:/
tests/20-
'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:/
tests/20-
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:/
tests/20-
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:/
tests/20-
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:/
tests/20-
On 2014/01/14 17:26:36, frankban wrote:
> These comments must go away.
gone
https:/