Sadly, my email response never made it through.  Here then is a cut/paste of
that message.

Note that I have a megamerge branch that encompasses the three outstanding
branches and fixes a few other things in this branch.

> Review: Needs Fixing code
>> =3D=3D=3D modified file 'src/lazr/config/README.txt'
>> --- src/lazr/config/README.txt	2008-12-15 22:22:12 +0000
>> +++ src/lazr/config/README.txt	2008-12-23 15:24:02 +0000
>> @@ -368,6 +368,23 @@
>>     # Accept the default values for the optional section-5.
>>     [section-5]
>>
>> +If the schema defines .master sections, then the conf file can =20
>> contain
>> +sections that extend the .master section.  These are like =20
>> categories with
>> +templates except that the section names extending .master need not =20=

>> be named in
>> +the schema file.
>
> I think we should explain why there is a master.
>
> {{
>
> The .master section allows admins to define configurations for an
> an arbitrary number of processes.
>
> }}

Good point, done.

>> +
>> +    >>> master_schema_conf =3D path.join(testfiles_dir,  =20
>> 'master.conf')
>> +    >>> master_local_conf =3D path.join(testfiles_dir,  'master-=20
>> local.conf')
>> +    >>> master_schema =3D ConfigSchema(master_schema_conf)
>> +    >>> len(master_schema.getByCategory('thing'))
>> +    0
>> +    >>> master_conf =3D master_schema.load(master_local_conf)
>> +    >>> sections =3D master_conf.getByCategory('thing')
>> +    >>> sorted(section.name for section in sections)
>> +    ['thing.one', 'thing.two']
>> +    >>> sorted(section.foo for section in sections)
>> +    ['1', '2']
>> +
>
> Does this work?
>
>>>> master_conf.thing.one.name
>    'thing.one'

It does! (now <wink>).

> ...
>
> =3D=3D=3D modified file 'src/lazr/config/__init__.py'
> --- src/lazr/config/__init__.py	2008-12-15 22:22:12 +0000
> +++ src/lazr/config/__init__.py	2008-12-23 15:24:02 +0000
> ...
>
>> @@ -87,9 +88,15 @@
>>         else:
>>             return (None, self.name)
>>
>> +    def clone(self):
>> +        """Return a copy of this section schema."""
>> +        return self.__class__(self.name, self._options.copy(),
>> +                              self.optional, self.master)
>> +
>
> This looks familiar
>
> /me looks.
>
> Oops! I see a nuance. This clone is fine. The clone I was thinking
> of still has James H's hack for mutable sections. They are no mutable,
> so the comment and sanity check can be removed.

Gone.

> ...
>
>> @@ -288,6 +296,7 @@
>>         name_parts =3D name.split('.')
>>         is_template =3D name_parts[-1] =3D=3D 'template'
>>         is_optional =3D name_parts[-1] =3D=3D 'optional'
>> +        is_master =3D name_parts[-1] =3D=3D 'master'
>
> In my design, .optional and .template were mutually exclusive. I =20
> believe
> this is true with .master.

Right.

> Your design implies that the schema may define a section from the =20
> master.
> I think this is practical, given that the application may require at =20=

> least
> one section defined for a process, but allow for additional =20
> processes to
> be defined in a config.

Yep.  I had some problems with that, but I've now fixed it.

>
> How does a section from a .master category interact with .optional? =20=

> May
> sections defined from a .master in a schema be optional? There
> are no tests for this. I believe this is legitimate in a schema
>
> {{{
>
> [whatsit.master]
> key: default
>
> [whatsit.always]
>
> [whatsit.sometimes.optional]
>
> }}}

It is legitimate, and with nothing in the config, this will work =20
exactly the same as templates.  Really, the way I think about it is =20
that a master is like a template except that additional sections can =20
be defined in the config as well as the schema.

> ...
>
>> @@ -344,6 +354,8 @@
>>         section_schemas =3D []
>>         for key in self._section_schemas:
>>             section =3D self._section_schemas[key]
>> +            if section.master:
>> +                continue
>
> I don't understand this. I expect .master sections to behave =20
> like .template
> sections.

Right!  Gone.

>>             category, dummy =3D section.category_and_section_names
>>             if name =3D=3D category:
>>                 section_schemas.append(section)
>
> ...
>
>> @@ -561,15 +573,34 @@
>>         errors =3D list(self.data._errors)
>>         errors.extend(encoding_errors)
>>         extends =3D None
>> +        masters =3D set()
>>         for section_name in parser.sections():
>>             if section_name =3D=3D 'meta':
>>                 extends, meta_errors =3D self._loadMetaData(parser)
>>                 errors.extend(meta_errors)
>>                 continue
>> -            if (section_name.endswith('.template')
>> -                or section_name.endswith('.optional')):
>> +            if (section_name.endswith('.template') or
>> +                section_name.endswith('.optional')):
>>                 # This section is a schema directive.
>>                 continue
>
> I have long wondered if this was bad. I now think it is better for
> and error to be collected when a config contains schema instructions.

It might be.  I didn't add it to this branch though.

> Regardless, of how we treat this error, .master should also be in the
> condition--defining a .master outside of the schema is insane because
> the application code must understand the primal definition.

Agreed.  I added the test for .master here so if we ever decide to =20
turn this into an exception, it will happen for all three types.

>> +            # Check for sections which extend .masters.
>> +            if '.' in section_name:
>> +                category, section =3D section_name.split('.')
>> +                master_name =3D category + '.master'
>> +                try:
>> +                    section_schema =3D self.schema[master_name]
>> +                except NoSectionError:
>> +                    # There's no master for this section.
>> +                    pass
>
> This comment is incomplete. The section must be defined from a =20
> template
>
> Oh! If I were to define a section from a .master in the schema, is it
> lost? Should there be a sanity check that the section is not already =20=

> defined?
> Does this mean that schema's cannot have sections defined =20
> from .masters?

Actually, they can.  There's a test for this now, and it works very =20
similarly to templates.

>> +                else:
>> +                    if section_schema.master:
>
> Is this test redundant? master name is like 'thing.master', so a =20
> section
> will be created with master set to True.

In fact, it is redundant.  I've replaced it with an assertion.  Trust =20=

but verify.

>> +                        schema =3D section_schema.clone()
>> +                        schema.name =3D section_name
>> +                        section =3D =20
>> self.schema.section_factory(schema)
>> +                        section.update(parser.items(section_name))
>> +                        sections[section_name] =3D section
>> +                        masters.add(master_name)
>> +                        continue
>

Here's the incremental.  Thanks for the review.
- -Barry

=3D=3D=3D modified file 'src/lazr/config/README.txt'
- --- src/lazr/config/README.txt	2008-12-23 14:33:21 +0000
+++ src/lazr/config/README.txt	2008-12-24 04:41:24 +0000
@@ -368,22 +368,26 @@
      # Accept the default values for the optional section-5.
      [section-5]

- -If the schema defines .master sections, then the conf file can =
contain
- -sections that extend the .master section.  These are like categories =20=

with
- -templates except that the section names extending .master need not be =
=20
named in
- -the schema file.
+The .master section allows admins to define configurations for an =20
arbitrary
+number of processes.  If the schema defines .master sections, then =20
the conf
+file can contain sections that extend the .master section.  These are =20=

like
+categories with templates except that the section names =20
extending .master need
+not be named in the schema file.

      >>> master_schema_conf =3D path.join(testfiles_dir,  =
'master.conf')
      >>> master_local_conf =3D path.join(testfiles_dir,  'master-=20
local.conf')
      >>> master_schema =3D ConfigSchema(master_schema_conf)
- -    >>> len(master_schema.getByCategory('thing'))
- -    0
+    >>> sections =3D master_schema.getByCategory('thing')
+    >>> sorted(section.name for section in sections)
+    ['thing.master']
      >>> master_conf =3D master_schema.load(master_local_conf)
      >>> sections =3D master_conf.getByCategory('thing')
      >>> sorted(section.name for section in sections)
      ['thing.one', 'thing.two']
      >>> sorted(section.foo for section in sections)
      ['1', '2']
+    >>> print master_conf.thing.one.name
+    thing.one

  The shared.conf file derives the keys and default values from the
  schema. This config was loaded before local.conf because its sections
@@ -793,21 +797,35 @@

  push() can also be used to extend master sections.

- -    >>> for section in master_conf.getByCategory('bar'):
+    >>> sections =3D sorted(master_conf.getByCategory('bar'),
+    ...                   key=3Dattrgetter('name'))
+    >>> for section in sections:
      ...     print section.name, section.baz
- -    bar.master 1
+    bar.master badger
+    bar.soup cougar

      >>> master_conf.push('override', """
      ... [bar.two]
- -    ... baz: 2
- -    ...
+    ... baz: dolphin
+    ... """)
+    >>> sections =3D sorted(master_conf.getByCategory('bar'),
+    ...                   key=3Dattrgetter('name'))
+    >>> for section in sections:
+    ...     print section.name, section.baz
+    bar.soup cougar
+    bar.two dolphin
+
+    >>> master_conf.push('overlord', """
      ... [bar.three]
- -    ... baz:3
+    ... baz: emu
      ... """)
- -    >>> for section in master_conf.getByCategory('bar'):
+    >>> sections =3D sorted(master_conf.getByCategory('bar'),
+    ...                   key=3Dattrgetter('name'))
+    >>> for section in sections:
      ...     print section.name, section.baz
- -    bar.two 2
- -    bar.three 3
+    bar.soup cougar
+    bar.three emu
+    bar.two dolphin


  pop()

=3D=3D=3D modified file 'src/lazr/config/__init__.py'
- --- src/lazr/config/__init__.py	2008-12-23 01:59:49 +0000
+++ src/lazr/config/__init__.py	2008-12-24 04:54:04 +0000
@@ -153,14 +153,7 @@
          The extension mechanism requires a copy of a section to =20
prevent
          mutation.
          """
- -        new_section =3D self.__class__(self.schema, =
self._options.copy())
- -        # XXX 2008-06-10 jamesh bug=3D237827:
- -        # Evil legacy code sometimes assigns directly to the config
- -        # section objects.  Copy those attributes over.
- -        new_section.__dict__.update(
- -            dict((key, value) for (key, value) in =20
self.__dict__.iteritems()
- -                 if key not in ['schema', '_options']))
- -        return new_section
+        return self.__class__(self.schema, self._options.copy())


  class ImplicitTypeSection(Section):
@@ -266,7 +259,7 @@
              (section_name, category_name,
               is_template, is_optional,
               is_master) =3D self._parseSectionName(name)
- -            if is_template:
+            if is_template or is_master:
                  templates[category_name] =3D dict(parser.items(name))
          for name in parser.sections():
              (section_name, category_name,
@@ -354,8 +347,6 @@
          section_schemas =3D []
          for key in self._section_schemas:
              section =3D self._section_schemas[key]
- -            if section.master:
- -                continue
              category, dummy =3D section.category_and_section_names
              if name =3D=3D category:
                  section_schemas.append(section)
@@ -580,7 +571,8 @@
                  errors.extend(meta_errors)
                  continue
              if (section_name.endswith('.template') or
- -                section_name.endswith('.optional')):
+                section_name.endswith('.optional') or
+                section_name.endswith('.master')):
                  # This section is a schema directive.
                  continue
              # Check for sections which extend .masters.
@@ -590,17 +582,18 @@
                  try:
                      section_schema =3D self.schema[master_name]
                  except NoSectionError:
- -                    # There's no master for this section.
+                    # There's no master for this section, so just =20
treat it
+                    # like a regular category.
                      pass
                  else:
- -                    if section_schema.master:
- -                        schema =3D section_schema.clone()
- -                        schema.name =3D section_name
- -                        section =3D =
self.schema.section_factory(schema)
- -                        section.update(parser.items(section_name))
- -                        sections[section_name] =3D section
- -                        masters.add(master_name)
- -                        continue
+                    assert section_schema.master, '.master is not a =20
master?'
+                    schema =3D section_schema.clone()
+                    schema.name =3D section_name
+                    section =3D self.schema.section_factory(schema)
+                    section.update(parser.items(section_name))
+                    sections[section_name] =3D section
+                    masters.add(master_name)
+                    continue
              if section_name not in self.schema:
                  # Any section not in the the schema is an error.
                  msg =3D "%s does not have a %s section." % (
@@ -616,9 +609,10 @@
              items =3D parser.items(section_name)
              section_errors =3D sections[section_name].update(items)
              errors.extend(section_errors)
- -        # Remove any master sections.
+        # master sections are like templates.  They show up in the =20
schema but
+        # not in the config.
          for master in masters:
- -            del sections[master]
+            sections.pop(master, None)
          return ConfigData(conf_name, sections, extends, errors)

      def _verifyEncoding(self, config_data):

=3D=3D=3D modified file 'src/lazr/config/testdata/master.conf'
- --- src/lazr/config/testdata/master.conf	2008-12-23 14:33:21 =
+0000
+++ src/lazr/config/testdata/master.conf	2008-12-24 04:41:24 =
+0000
@@ -1,6 +1,9 @@
  # This section defines a category master.
  [thing.master]
- -foo: 0
+foo: aardvark

  [bar.master]
- -baz: 1
+baz: badger
+
+[bar.soup]
+baz: cougar