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

Cleanups in e2e retries #1599

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Cleanups in e2e retries #1599

wants to merge 4 commits into from

Conversation

nirs
Copy link
Member

@nirs nirs commented Oct 22, 2024

  • Use time.Duration for timeouts
  • Rename TimeoutInterval to RetryInterval
  • Shorten retry internal to 5 seconds
  • Remove unneeded use of fmt.Sprintf

We use untyped value with a // seconds comment, and then converted the
untyped value to time.Duration everywhere. Use time.Duration to make the
code more clear and less error prone.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
The original name was not clear but we called it Retry Interval when
logging the config.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
5 seconds delay is large enough to avoid unwanted load on the
environment, and short enough to detect changes quickly. With this
change short operations that took more than 30 seconds before take now
about 5 seconds.

This also simplifies the retry loops to used special 5 seconds sleep
before retrying. Now we check immediately since we will retry after 5
seconds anyway.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
When using fmt.Errorf there is no need to format the error message.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
@nirs nirs marked this pull request as draft October 22, 2024 18:50
@nirs nirs changed the title E2e retry Cleanups in e2e retries Oct 22, 2024
@nirs nirs marked this pull request as ready for review October 22, 2024 19:19
@nirs nirs marked this pull request as draft October 25, 2024 09:46
@nirs
Copy link
Member Author

nirs commented Oct 25, 2024

Needs rebase

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.

1 participant