Merge lp://qastaging/~vbocek/ubuntu-system-image/support_http into lp://qastaging/~registry/ubuntu-system-image/client

Just a quick comment as I'm going through your branch.

=== modified file 'systemimage/config.py'
--- systemimage/config.py 2014-02-13 18:16:17 +0000
+++ systemimage/config.py 2014-02-25 22:24:24 +0000
> @@ -64,32 +78,39 @@
> if files_read != [path]:
> raise FileNotFoundError(path)
> self.config_file = path
> - self.service = Bag(converters=dict(http_port=int,
> - https_port=int,
> + self.service.update(converters=dict(http_port=port_value_converter,
> + https_port=port_value_converter,
> build_number=int),
> **parser['service'])
> + if (self.service.http_port is DISABLED_PROTOCOL and
> + self.service.https_port is DISABLED_PROTOCOL):
> + raise ValueError('both http and https ports were disabled')
> # Construct the HTTP and HTTPS base urls, which most applications will
> # actually use.
> if self.service.http_port == 80:
> self.service['http_base'] = 'http://{}'.format(self.service.base)
> - else:
> + elif self.service.http_port is not DISABLED_PROTOCOL:
> self.service['http_base'] = 'http://{}:{}'.format(
> self.service.base, self.service.http_port)
> if self.service.https_port == 443:
> self.service['https_base'] = 'https://{}'.format(self.service.base)
> - else:
> + elif self.service.http_port is not DISABLED_PROTOCOL:
> self.service['https_base'] = 'https://{}:{}'.format(
> self.service.base, self.service.https_port)
> - # Short-circuit, since we're loading a channel.ini file.
> - self._override = override
> - if override:
> - return
> - self.system = Bag(converters=dict(timeout=as_timedelta,
> + if self.service.http_port is DISABLED_PROTOCOL:
> + self.service['http_base'] = self.service['https_base']
> + elif self.service.https_port is DISABLED_PROTOCOL:
> + self.service['https_base'] = self.service['http_base']
> + self.system.update(converters=dict(timeout=as_timedelta,
> build_file=expand_path,
> loglevel=as_loglevel,
> settings_db=expand_path,
> tempdir=expand_path),
> **parser['system'])

The changes above are problematic because currently, Bags have the semantic
that their values cannot change after the initial setting. So updating as is
done here raises a ValueError. It's worse too because the test infrastructure
requires a Configuration object before any tests run (for unfortunate reasons
having to do with the D-Bus tests).

I'm not exactly sure of the best way to resolve this. I should probably just
remove this read-only restriction, since it's not really serving a useful
purpose.

On Feb 26, 2014, at 12:03 AM, Barry Warsaw wrote:

>I'm not exactly sure of the best way to resolve this. I should probably just
>remove this read-only restriction, since it's not really serving a useful
>purpose.

nm, I know how to resolve this.

It didn't raise any error while I was testing it, the exception in __setitem__ affects only external changes via [] operator.

>>> from bag import Bag
>>> b=Bag(a=4, b=8)
>>> b.update(a=0, b=2)
>>> b['a']
0
>>> b['b']
2

Thanks very much for this branch. I'm going to merge it to trunk after some
tweaks, and fleshing out of the test suite.

I ended up making these changes to your branch:

  * Renamed DISABLED_PROTOCOL to DISABLED
  * Some minor code style issues.
  * Allow for the [system] section to be missing from a channel.ini file.
  * Bag class API change: Setting an attribute via the setitem API no longer
    makes that item read-only. IOW, only keywords set in the constructor are
    read-only.
  * Flesh out tests.
  * Tweak the manpage.

Once I've merged this to trunk, you can take a look at the specific diff
there. Please let me know if I've missed anything or if you have any
questions or comments.

Approved, with tweaks. See lp:~barry/ubuntu-system-image/lp1278589