cloud-init-dev team mailing list archive

[Scott Moser <smoser@xxxxxxxxxx>]

i approve with 2 small questions in line.


Diff comments:

> diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py
> index 88cb7f8..de459c9 100644
> --- a/cloudinit/user_data.py
> +++ b/cloudinit/user_data.py
> @@ -222,16 +223,25 @@ class UserDataProcessor(object):
>              if include_once_on and os.path.isfile(include_once_fn):
>                  content = util.load_file(include_once_fn)
>              else:
> -                resp = util.read_file_or_url(include_url,
> -                                             ssl_details=self.ssl_details)
> -                if include_once_on and resp.ok():
> -                    util.write_file(include_once_fn, resp.contents, mode=0o600)
> -                if resp.ok():
> -                    content = resp.contents
> -                else:
> -                    LOG.warning(("Fetching from %s resulted in"
> -                                 " a invalid http code of %s"),
> -                                include_url, resp.code)
> +                try:
> +                    resp = util.read_file_or_url(include_url,
> +                                                 ssl_details=self.ssl_details)
> +                    if include_once_on and resp.ok():
> +                        util.write_file(include_once_fn, resp.contents,
> +                                        mode=0o600)
> +                    if resp.ok():
> +                        content = resp.contents
> +                    else:
> +                        LOG.warning(("Fetching from %s resulted in"
> +                                     " a invalid http code of %s"),
> +                                    include_url, resp.code)
> +                except UrlError as urle:
> +                    LOG.warning(
> +                        "Fetching from %s resulted in a UrlError: %s",
> +                        include_url, urle.cause)

i think just printing urle here in its string form should be reasonable.
str(urle) would look like:
 cloudinit.url_helper.UrlError('404 Client Error: Not Found for url: http://example.com/asdf')
str(urle.cause) would look  like:
 requests.exceptions.HTTPError('404 Client Error: Not Found for url: http://example.com/asdf')

> +                except IOError as ioe:
> +                    LOG.warning("Fetching from %s resulted in an IOError: %s",
> +                                include_url, ioe.strerror)

i guess fine, but is there a reason for 'ioe.strerror' rather than just str(ioe) ?

>  
>              if content is not None:
>                  new_msg = convert_string(content)


-- 
https://code.launchpad.net/~ajorgens/cloud-init/+git/cloud-init/+merge/331660
Your team cloud-init commiters is requested to review the proposed merge of ~ajorgens/cloud-init:_include-urlerror into cloud-init:master.