← Back to team overview

curtin-dev team mailing list archive

Re: [Merge] ~mwhudson/curtin:extract-refactor into curtin:master

 

Review: Approve

Given the first comment I particularly went looking for things that might need a comment, but honestly it seems clear to me.  I had one very minor comment suggestion.

Diff comments:

> diff --git a/curtin/commands/extract.py b/curtin/commands/extract.py
> index 069023f..d920d50 100644
> --- a/curtin/commands/extract.py
> +++ b/curtin/commands/extract.py
> @@ -186,21 +152,44 @@ def _get_image_stack(uri):
>      '''
>  
>      image_stack = []
> -    root_dir = os.path.dirname(uri)
>      img_name = os.path.basename(uri)
> -    _, img_ext = os.path.splitext(img_name)
> +    root_dir = uri[:-len(img_name)]
> +    img_base, img_ext = os.path.splitext(img_name)
>  
> -    img_parts = img_name.split('.')
> -    # Last item is the extension
> -    for i in img_parts[:-1]:
> +    if not img_base:
> +        return []
> +
> +    img_parts = img_base.split('.')
> +    for i in range(len(img_parts)):
>          image_stack.append(
> -            os.path.join(
> -                root_dir,
> -                '.'.join(img_parts[0:img_parts.index(i)+1]) + img_ext))
> +            root_dir + '.'.join(img_parts[0:i+1]) + img_ext)
>  
>      return image_stack
>  
>  
> +def get_image_handler_for_source(source):

Nitpick: We could document here that the caller is expected to call cleanup, but I'm not sure anyone will be calling to get_image_hander_for_source() directly.

> +    if source['uri'].startswith("cp://"):
> +        return TrivialImageHandler(source['uri'])
> +    elif source['type'] == "fsimage":
> +        return LayeredImageHandler([source['uri']])
> +    elif source['type'] == "fsimage-layered":
> +        return LayeredImageHandler(_get_image_stack(source['uri']))
> +    else:
> +        return None
> +
> +
> +def extract_source(source, target):
> +    handler = get_image_handler_for_source(source)
> +    if handler is not None:
> +        root_dir = handler.setup()
> +        try:
> +            copy_to_target(root_dir, target)
> +        finally:
> +            handler.cleanup()
> +    else:
> +        extract_root_tgz_url(source['uri'], target=target)
> +
> +
>  def copy_to_target(source, target):
>      if source.startswith("cp://"):
>          source = source[5:]


-- 
https://code.launchpad.net/~mwhudson/curtin/+git/curtin/+merge/410267
Your team curtin developers is subscribed to branch curtin:master.



References