Code review comment for lp:~adam-collard/landscape-client/juju-info-at-registration

Looks good, nothing major, but still marking it as Needs Fixing,
since [6] might change the diff a bit.

[1]

29 + self._juju_data = self._get_juju_data()

I think it would be cleaner to collect the data when on "run", just
like ec2 data is collected.

[2]

222 + self.assertMessages(self.mstore.get_pending_messages(),
223 + [{"type": "register",
224 + "computer_title": self.config.computer_title,
225 + "account_name": "account_name",
226 + "registration_password": None,
227 + "hostname": socket.getfqdn(),
228 + "vm-info": get_vm_info(),
229 + "tags": None,
230 + "juju-info": {
231 + "environment-uuid": "DEAD-BEEF",
232 + "api-addresses": ["10.0.3.1:17070"],
233 + "unit-name": "juju-unit-name"}
234 + }])

The indentation looks off here. Also, please use valid unit names,
like service/0.

[3]
264 - "tags": None}])
265 + "tags": None,
266 + "juju-info": {
267 + "environment-uuid": "DEAD-BEEF",
268 + "api-addresses": ["10.0.3.1:17070"],
269 + "unit-name": "juju-unit-name"}}])

Indentation is off here as well.

[4]
365 + def test_get_juju_info_sample_data(self):
366 + """L{get_juju_info} works with sample data."""

What does "works with sample data" mean? It's implied that our
code works. Better to explain what the code is expected to do instead.

[5]
384 + self.assertTrue(
385 + "Error attempting to read JSON" in self.logfile.getvalue())

Please use assertIn instead, which gives a better error message on failures.

[6]

435 + "juju-info": KeyDict(juju_data)},
436 + optional=["tags", "vm-info", "public_ipv4", "local_ipv4",
437 + "juju-info"])

I think we shouldn't include juju-info in the register-cloud-vm message.
By default Juju instances won't use that the register-cloud-vm message,
and the plan is to remove it altogether.