← Back to team overview

cloud-init-dev team mailing list archive

Re: [Merge] ~smoser/cloud-init:cleanup/consistent-exception_cb into cloud-init:master

 


Diff comments:

> diff --git a/cloudinit/ec2_utils.py b/cloudinit/ec2_utils.py
> index dc3f0fc..18553f0 100644
> --- a/cloudinit/ec2_utils.py
> +++ b/cloudinit/ec2_utils.py
> @@ -134,9 +134,9 @@ class MetadataMaterializer(object):
>          return joined
>  
>  
> -def _skip_retry_on_codes(status_codes, _request_args, cause):
> -    """Returns False if cause.code is in status_codes."""
> -    return cause.code not in status_codes
> +def _raise_cause_if_not_found(status_codes, _request_args, cause):

This is not a valid exception_callback as it's expecting 3 params) and exception_cb only calls with the 2 params (args, exception). I think we just need to drop status_codes param.

> +    if cause.code not in SKIP_USERDATA_CODES:
> +        raise cause
>  
>  
>  def get_instance_userdata(api_version='latest',
> diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
> index 0ee622e..b2244f5 100644
> --- a/cloudinit/sources/DataSourceAzure.py
> +++ b/cloudinit/sources/DataSourceAzure.py
> @@ -452,10 +452,10 @@ class DataSourceAzure(sources.DataSource):
>  
>          def exc_cb(msg, exception):

msg is actually a dict of args, we should probably clean this up in all datasource callsites to avoid ambiguity

>              if isinstance(exception, UrlError) and exception.code == 404:
> -                return True
> +                raise exception
>              # If we get an exception while trying to call IMDS, we
>              # call DHCP and setup the ephemeral network to acquire the new IP.
> -            return False
> +            return
>  
>          need_report = report_ready
>          while True:
> diff --git a/cloudinit/sources/DataSourceScaleway.py b/cloudinit/sources/DataSourceScaleway.py
> index e2502b0..a89e125 100644
> --- a/cloudinit/sources/DataSourceScaleway.py
> +++ b/cloudinit/sources/DataSourceScaleway.py
> @@ -103,21 +103,25 @@ def query_data_api_once(api_address, timeout, requests_session):
>      ports below 1024). If requests raises ConnectionError (EADDRINUSE), the
>      caller should retry to call this function on an other port.
>      """
> +    def _exc_cb(msg, exception):

should be args instead of msg for the first parameter to avoid ambiguity.

> +        """raise the provided exception if it is not 404."""
> +        if (exception.code == 404 or
> +                isinstance(exception.cause,
> +                           requests.exceptions.ConnectionError)):
> +            raise exception
> +
>      try:
>          resp = url_helper.readurl(
>              api_address,
>              data=None,
>              timeout=timeout,
> -            # It's the caller's responsability to recall this function in case
> +            # It's the caller's responsibility to recall this function in case
>              # of exception. Don't let url_helper.readurl() retry by itself.
>              retries=0,
>              session=requests_session,
>              # If the error is a HTTP/404 or a ConnectionError, go into raise
>              # block below and don't bother retrying.
> -            exception_cb=lambda _, exc: exc.code != 404 and (
> -                not isinstance(exc.cause, requests.exceptions.ConnectionError)
> -            )
> -        )
> +            exception_cb=_exc_cb)
>          return util.decode_binary(resp.contents)
>      except url_helper.UrlError as exc:
>          # Empty user data.


-- 
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/342007
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:cleanup/consistent-exception_cb into cloud-init:master.


References