Skip to content

sources/azure: refactor http_with_retries to remove **kwargs#1392

Merged
blackboxsw merged 2 commits into
canonical:mainfrom
cjp256:azure-refactor-http-with-retries-kwargs
Apr 20, 2022
Merged

sources/azure: refactor http_with_retries to remove **kwargs#1392
blackboxsw merged 2 commits into
canonical:mainfrom
cjp256:azure-refactor-http-with-retries-kwargs

Conversation

@cjp256
Copy link
Copy Markdown
Contributor

@cjp256 cjp256 commented Apr 19, 2022

Remove unused functionality and general kwargs support in favor
of defined parameters.

As only headers and data kwargs are used, add those explicitly
to http_with_retries().

There should be no behavioral changes.

Signed-off-by: Chris Patterson cpatterson@microsoft.com

Remove unused functionality and general kwargs support in favor
of defined parameters.

As only headers and data kwargs are used, add those explicitly
to http_with_retries().

There should be no behavioral changes.

Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
Copy link
Copy Markdown
Contributor

@anhvoms anhvoms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread cloudinit/sources/helpers/azure.py Outdated
@azure_ds_telemetry_reporter
def http_with_retries(url, **kwargs) -> url_helper.UrlResponse:
def http_with_retries(
url, *, headers: dict, data=None
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are type hinting, let's get url too

Suggested change
url, *, headers: dict, data=None
url: str, *, headers: dict, data=None

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, for data too, thanks!

Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a minor nit and +1

@cjp256 cjp256 force-pushed the azure-refactor-http-with-retries-kwargs branch from 21836ca to e5706fb Compare April 20, 2022 11:51
Signed-off-by: Chris Patterson <cpatterson@microsoft.com>
Copy link
Copy Markdown
Collaborator

@blackboxsw blackboxsw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cjp256

@blackboxsw blackboxsw merged commit fbb4ab0 into canonical:main Apr 20, 2022
@cjp256 cjp256 deleted the azure-refactor-http-with-retries-kwargs branch April 25, 2022 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants