Code review comment for lp:~hazmat/pyjuju/local-provider-config

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

LGTM, with a few details:

[1]

> provider: lxc

It feels like a bad idea to be naming the local provider as "lxc", in hindsight. It conflates
two different concepts: deployment of units in lxc, that can happen in any provider type,
with the use of the local machine for development purposes, which is what this setting is
really about.

Can we please rename the provider itself to "local" on a follow up branch once all
the in-progress work is merged (to avoid having to deal with conflicts)?

[2]

27 +ensemble: environments
28 +

All yaml headers were dropped. Please kill this before merging.

[3]

66 def __init__(self):
67 - self.instance_id = "local"
68 - self.private_dns_name = self.dns_name = "localhost"
69 + pass

I'd prefer to use the constructor of the base class instead, as
this will hook up both implementations and we'll get to know
about divergences:

super(LocalMachine, self).__init__("local", private_dns_name="localhost")

review: Approve

« Back to merge proposal