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

chore: add retry to tests requiring forwarding #949

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

jjaakola-aiven
Copy link
Contributor

About this change - What it does

References: #xxxxx

Why this way

@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch 4 times, most recently from 4b6872c to 2ae1e2e Compare September 18, 2024 06:22
Copy link

github-actions bot commented Sep 18, 2024

Coverage report

This PR does not seem to contain any modification to coverable code.

@jjaakola-aiven jjaakola-aiven marked this pull request as ready for review September 18, 2024 07:43
@jjaakola-aiven jjaakola-aiven requested review from a team as code owners September 18, 2024 07:43
@nosahama nosahama force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch from 9f811ef to bc283b3 Compare September 18, 2024 09:24
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch from bc283b3 to e8792b2 Compare September 18, 2024 09:24
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch from e8792b2 to 7b43c41 Compare September 18, 2024 09:57
nosahama
nosahama previously approved these changes Sep 18, 2024
tests/integration/utils/rest_client.py Outdated Show resolved Hide resolved
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch 2 times, most recently from 99712fa to 2fe7d3d Compare September 18, 2024 10:22
keejon
keejon previously approved these changes Sep 18, 2024
Copy link
Contributor

@keejon keejon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@keejon keejon enabled auto-merge September 18, 2024 10:25
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch 4 times, most recently from 773afa8 to 0d2c3ac Compare September 19, 2024 05:56
@nosahama nosahama force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch from 0d2c3ac to b0916cf Compare September 19, 2024 09:27
# Test primary/replica forwarding with global config setting
primary_url, replica_url = registry_async_auth_pair
max_tries, counter = 5, 0
wait_time = 0.5
for compat in ["FULL", "BACKWARD", "FORWARD", "NONE"]:
resp = requests.put(f"{replica_url}/config", json={"compatibility": compat}, auth=auth)
resp = await registry_async_retry_client_auth.put(f"{replica_url}/config", json={"compatibility": compat}, auth=admin)
Copy link
Contributor

Choose a reason for hiding this comment

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

This was failing and we had to split the used auth, aiohttp requires the BasicAuth and requests requires something different, but we were first passing the requests auth to the aiohttp client and it was failing.

@nosahama nosahama force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch from b0916cf to 09f06a1 Compare September 19, 2024 09:32
@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch from 09f06a1 to 9051154 Compare September 19, 2024 10:24
Copy link
Contributor

@AnatolyPopov AnatolyPopov left a comment

Choose a reason for hiding this comment

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

LGTM

@jjaakola-aiven jjaakola-aiven force-pushed the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch from 9051154 to 86c3817 Compare September 19, 2024 11:07
Copy link
Contributor

@keejon keejon left a comment

Choose a reason for hiding this comment

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

👍

@keejon keejon merged commit b0bd72d into main Sep 19, 2024
10 checks passed
@keejon keejon deleted the jjaakola-aiven-add-retry-to-tests-requiring-forwarding branch September 19, 2024 13:01
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.

4 participants