Comment 4 for bug 341705

[1]
- args, accept_unexistent_config=accept_unexistent_config)
+ args, accept_nonexistent_config=accept_nonexistent_config)

Thanks :-)

[2]
+ def __init__(self, fetch_import_url=None):
+ super(LandscapeSetupConfiguration, self).__init__()
+ if fetch_import_url is None:
+ self._fetch_import_url = self._undefined_fetch_import_url
+ else:
+ self._fetch_import_url = fetch_import_url
+
+ def _undefined_fetch_import_url(self, url):
+ raise RuntimeError("fetch_import_url not defined")

This code seems unnecessary and indeed it's untested. If I just change it to unconditionally say "self._fetch_import_url = fetch_import_url", all the tests still pass.

[3]
+ def _load_external_options(self):
+ if self.import_from:
+ parser = ConfigParser()
+
+ # Imported options behave as if they were passed in the
+ # command line, with precedence being given to real command
+ # line options.

That comment seems like it'd be better in a docstring or even in the --help for the option, since it's actually something that's important for the user to know.

[4]
+ if config.ssl_public_key and config.ssl_public_key[:7] == "base64:":

There's a neat new feature in python 1.6 called str.startswith ;-)

[5] Could you add a test for importing an URL with outright syntax errors? Like, for example, if someone tries to --import http://google.com/, what would happen?

[6] This does seem to be handled correctly, but could you add an explicit test that when "nulling out" a value in the imported configuration file, it correctly saves the empty value? i.e. if old config has "registration_password = foo" and imported configuration has "registration_password = ", then the new config should have "" as the value for registration_password.

This is looking good! +1 with these changes.

Also, I think we should get Andreas to take a crack at this branch before we merge it.