Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

builder: add retries to composer API calls #103

Merged
merged 1 commit into from
Aug 5, 2022
Merged

Conversation

ondrejbudai
Copy link
Member

The status calls are sometimes failing on:
upstream connect error or disconnect/reset before headers. reset reason:
connection termination

Since all requests are going through the company proxy, I think that the
networking isn't working 100% reliably. This commit adds a retry mechanism
provided by the urllib3 library. It will retry on all networking issues and
also on some 5xx errors that makes sense to retry (like gateway failures).

A test is added that runs the compose waiting code against a mock server
that fails every second request. This is imho sufficient to mimick a flaky
networking.

Copy link
Contributor

@gicmo gicmo left a comment

Choose a reason for hiding this comment

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

There are a bunch of unrelated whitespace and import reordering changes.

The status calls are sometimes failing on:
upstream connect error or disconnect/reset before headers. reset reason:
connection termination

Since all requests are going through the company proxy, I think that the
networking isn't working 100% reliably. This commit adds a retry mechanism
provided by the urllib3 library. It will retry on all networking issues and
also on some 5xx errors that makes sense to retry (like gateway failures).

A test is added that runs the compose waiting code against a mock server
that fails every second request. This is imho sufficient to mimick a flaky
networking.
@ondrejbudai
Copy link
Member Author

These IDEs are smart... Fixed, thanks for noticing it! :)

@ondrejbudai ondrejbudai requested a review from gicmo July 20, 2022 12:58
Copy link
Contributor

@gicmo gicmo left a comment

Choose a reason for hiding this comment

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

LGTM

@gicmo gicmo merged commit d38e11e into osbuild:main Aug 5, 2022
@teg
Copy link
Member

teg commented Aug 5, 2022

How long (in seconds) will we retry for? In fedora Koji we may have outages of up to 30sec.

@gicmo
Copy link
Contributor

gicmo commented Aug 5, 2022

How long (in seconds) will we retry for? In fedora Koji we may have outages of up to 30sec.

backoff_factor ([float](https://docs.python.org/3/library/functions.html#float)) –
A backoff factor to apply between attempts after the second try (most errors are resolved immediately by a second try without a delay). urllib3 will sleep for:

{backoff factor} * (2 ** ({number of total retries} - 1))

seconds. If the backoff_factor is 0.1, then [Retry.sleep()](https://urllib3.readthedocs.io/en/latest/reference/urllib3.util.html#urllib3.util.Retry.sleep) will sleep for [0.0s, 0.2s, 0.4s, …] between retries. It will never be longer than backoff_max.

By default, backoff is disabled (set to 0).

@teg
Copy link
Member

teg commented Aug 5, 2022

Thanks, won't really make a difference for he firewall issue then.

@gicmo
Copy link
Contributor

gicmo commented Aug 5, 2022

Could make it a config option. Or just increase it?

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