Re: [layerindex-web][PATCH 1/1] import_layer.py: add -t option for layer_type

[Paul Eggleton <paul.eggleton@linux.intel.com>]

Hi Robert

Sorry I meant to reply earlier.

I'm not especially happy with moving the constants to utils.py. Is there any 
way we can avoid that?

Thanks
Paul

On Thursday, 30 August 2018 2:05:11 PM NZST Robert Yang wrote:
> Hi Paul,
> 
> Do you have any comments on this, please ?
> 
> // Robert
> 
> On 07/31/2018 03:57 PM, Robert Yang wrote:
> > Hi Paul,
> > 
> > On 07/24/2018 04:33 PM, Paul Eggleton wrote:
> >> Hi Robert,
> >>
> >> On Monday, 23 July 2018 12:30:01 PM CEST Robert Yang wrote:
> >>> Now the logic is:
> >>> Use options.layer_type if specified, guess if not, default to 'M'.
> >>>
> >>> Note choices=['A', 'B', 'S', 'D', 'M', ''], the '' is for default='', we 
can't
> >>> use default='M' here, otherwise we don't know whether the 'M' is 
specified by
> >>> user or not, we don't guess if it is specified by user, otherwise, 
guess.
> >>>
> >>> Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> >>> ---
> >>>   layerindex/tools/import_layer.py | 18 +++++++++++++-----
> >>>   1 file changed, 13 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/layerindex/tools/import_layer.py b/layerindex/tools/
import_layer.py
> >>> index 2413cff..0d501f6 100755
> >>> --- a/layerindex/tools/import_layer.py
> >>> +++ b/layerindex/tools/import_layer.py
> >>> @@ -189,6 +189,10 @@ def main():
> >>>       parser.add_option("-s", "--subdir",
> >>>               help = "Specify subdirectory",
> >>>               action="store", dest="subdir")
> >>> +    parser.add_option("-t", "--type",
> >>> +            help = "Specify layer type. A: Base, B: Machine(BSP), S: 
> >>> Software, D: Distribution, M: Miscellaneous",
> >>> +            choices=['A', 'B', 'S', 'D', 'M', ''],
> >>> +            action="store", dest="layer_type", default='')
> >>
> >> If it's practical to do, could you make this part of the code use
> >> LayerItem.LAYER_TYPE_CHOICES from models.py so that this will work
> >> if that is extended in future? Of course that will mean initialising 
Django
> >> earlier, I'm not sure if that will have any side-effects that we wouldn't 
want.
> >> If you could give that a try though that would be great.
> > 
> > I think that the easies way is move the definition of LAYER_TYPE_CHOICES
> > to utils.py, then we can use utils.LAYER_TYPE_CHOICES in both models.py
> > and tools/import_layer.py
> > 
> > I've updated it in the repo:
> > 
> >    git://git.pokylinux.org/poky-contrib rbt/li
> >    http://git.pokylinux.org/cgit.cgi//log/?h=rbt/li
> > 
> > commit aeb1fd6a8f10b40637121643fc7df2f17c9cd87c
> > Author: Robert Yang <liezhi.yang@windriver.com>
> > Date:   Mon Jul 9 15:28:10 2018 +0800
> > 
> >      import_layer.py: add -t option for layer_type
> > 
> >      Now the logic is:
> >      Use options.layer_type if specified, and guess if not. Default to 
'M'.
> > 
> >      Note choices=['A', 'B', 'S', 'D', 'M', ''], the '' is for default='', 
we can't
> >      use default='M' here, otherwise we don't know whether the 'M' is 
specified by
> >      user or is the default value, we don't guess if it is specified by 
user,
> >      otherwise, guess.
> > 
> >      Signed-off-by: Robert Yang <liezhi.yang@windriver.com>
> > 
> > diff --git a/layerindex/models.py b/layerindex/models.py
> > index e5c8052..1ff653f 100644
> > --- a/layerindex/models.py
> > +++ b/layerindex/models.py
> > @@ -102,14 +102,8 @@ class LayerItem(models.Model):
> >           ('P', 'Published'),
> >           ('X', 'No update'),
> >       )
> > -    LAYER_TYPE_CHOICES = (
> > -        ('A', 'Base'),
> > -        ('B', 'Machine (BSP)'),
> > -        ('S', 'Software'),
> > -        ('D', 'Distribution'),
> > -        ('W', 'WRTemplates'),
> > -        ('M', 'Miscellaneous'),
> > -    )
> > +    LAYER_TYPE_CHOICES = utils.LAYER_TYPE_CHOICES
> > +
> >       name = models.CharField('Layer name', max_length=40, unique=True, 
> > help_text='Name of the layer - must be unique and can only contain 
letters, 
> > numbers and dashes')
> >       status = models.CharField(max_length=1, 
choices=LAYER_STATUS_CHOICES, 
> > default='N')
> >       layer_type = models.CharField(max_length=1, 
choices=LAYER_TYPE_CHOICES)
> > diff --git a/layerindex/tools/import_layer.py b/layerindex/tools/
import_layer.py
> > index 2413cff..5373cce 100755
> > --- a/layerindex/tools/import_layer.py
> > +++ b/layerindex/tools/import_layer.py
> > @@ -178,6 +178,21 @@ def get_github_layerinfo(layer_url, username = None, 
> > password = None):
> > 
> >       return (json_data, owner_json_data)
> > 
> > +def get_layer_type_choices():
> > +    """
> > +    Return help string and choices for --type.
> > +    """
> > +    help_str = "Specify layer type."
> > +    choices = []
> > +    for i in utils.LAYER_TYPE_CHOICES:
> > +        key, description = i
> > +        help_str += ' %s: %s,' % (key, description)
> > +        choices.append(key)
> > +
> > +    help_str = help_str.rstrip(',')
> > +    choices.append('')
> > +
> > +    return (help_str, choices)
> > 
> >   def main():
> >       valid_layer_name = re.compile('[-\w]+$')
> > @@ -186,9 +201,15 @@ def main():
> >           usage = """
> >       %prog [options] <url> [name]""")
> > 
> > +    layer_type_help, layer_type_choices = get_layer_type_choices()
> > +
> >       parser.add_option("-s", "--subdir",
> >               help = "Specify subdirectory",
> >               action="store", dest="subdir")
> > +    parser.add_option("-t", "--type",
> > +            help = layer_type_help,
> > +            choices = layer_type_choices,
> > +            action="store", dest="layer_type", default='')
> >       parser.add_option("-n", "--dry-run",
> >               help = "Don't write any data back to the database",
> >               action="store_true", dest="dryrun")
> > @@ -263,7 +284,6 @@ def main():
> >               layer = LayerItem()
> >               layer.name = layer_name
> >               layer.status = 'P'
> > -            layer.layer_type = 'M'
> >               layer.summary = 'tempvalue'
> >               layer.description = layer.summary
> > 
> > @@ -349,11 +369,18 @@ def main():
> > 
> > 
> >                   logger.info('Creating layer %s' % layer.name)
> > -                # Guess layer type
> > -                if glob.glob(os.path.join(layerdir, 'conf/distro/
*.conf')):
> > +                # Guess layer type if not specified
> > +                if options.layer_type:
> > +                    layer.layer_type = options.layer_type
> > +                elif layer.name in ['openembedded-core', 'meta-oe']:
> > +                    layer.layer_type = 'A'
> > +                elif glob.glob(os.path.join(layerdir, 'conf/distro/
*.conf')):
> >                       layer.layer_type = 'D'
> >                   elif glob.glob(os.path.join(layerdir, 'conf/machine/
*.conf')):
> >                       layer.layer_type = 'B'
> > +                else:
> > +                    layer.layer_type = 'M'
> > +
> >                   layer.save()
> >                   layerbranch = LayerBranch()
> >                   layerbranch.layer = layer
> > @@ -411,11 +438,9 @@ def main():
> > 
> >                   if layer.name == 'openembedded-core':
> >                       layer.summary = 'Core metadata'
> > -                    layer.layer_type = 'A'
> >                   elif layer.name == 'meta-oe':
> >                       layer.summary = 'Additional shared OE metadata'
> >                       layer.description = layer.summary
> > -                    layer.layer_type = 'A'
> > 
> >                   if maintainers:
> >                       maint_re = re.compile(r'^"?([^"@$<>]+)"? *<([^<> ]
+)>[ 
> > -]*(.+)?$')
> > diff --git a/layerindex/utils.py b/layerindex/utils.py
> > index 861a841..5f44754 100644
> > --- a/layerindex/utils.py
> > +++ b/layerindex/utils.py
> > @@ -15,6 +15,15 @@ import fcntl
> >   import signal
> >   import codecs
> > 
> > +LAYER_TYPE_CHOICES = (
> > +    ('A', 'Base'),
> > +    ('B', 'Machine (BSP)'),
> > +    ('S', 'Software'),
> > +    ('D', 'Distribution'),
> > +    ('W', 'WRTemplates'),
> > +    ('M', 'Miscellaneous'),
> > +)
> > +
> >   def get_branch(branchname):
> >       from layerindex.models import Branch
> >       res = list(Branch.objects.filter(name=branchname)[:1])
> > 
> > // Robert
> >>
> >> Thanks,
> >> Paul
> >>
> 


-- 

Paul Eggleton
Intel Open Source Technology Centre