Skip to content

AWS: Fix flaky TestS3RestSigner#10898

Merged
nastra merged 1 commit intoapache:mainfrom
nastra:fix-tests3restsigner
Aug 8, 2024
Merged

AWS: Fix flaky TestS3RestSigner#10898
nastra merged 1 commit intoapache:mainfrom
nastra:fix-tests3restsigner

Conversation

@nastra
Copy link
Contributor

@nastra nastra commented Aug 7, 2024

fixes #10599

@nastra nastra requested a review from amogh-jahagirdar August 7, 2024 10:56
@github-actions github-actions bot added the AWS label Aug 7, 2024
Copy link
Contributor

@amogh-jahagirdar amogh-jahagirdar left a comment

Choose a reason for hiding this comment

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

Overall LGTM, just had a comment on clarifying the code comment a bit more.
It'd be ideal to be able to not have to rely on timing but I also want to unblock flaky tests that others may be hitting on the PR and since this is a test we can always change it later if we are able to get a better way of testing what we want to test.

@nastra nastra force-pushed the fix-tests3restsigner branch from 29d7f3a to e9d86fc Compare August 8, 2024 07:00
@nastra
Copy link
Contributor Author

nastra commented Aug 8, 2024

It'd be ideal to be able to not have to rely on timing but I also want to unblock flaky tests that others may be hitting on the PR and since this is a test we can always change it later if we are able to get a better way of testing what we want to test.

Yes I agree, we don't want to rely on the timing of the test class itself and typically running this test locally took < 10 seconds so I initially just picked 100 seconds for the token expiration, which on the other hand didn't seem enough for CI.
In the end, all we care about in this particular scenario is that there's only a single token left in the refresh scheduler's queue in order to make sure #7270 doesn't happen

@nastra nastra merged commit 3bee806 into apache:main Aug 8, 2024
@nastra nastra deleted the fix-tests3restsigner branch August 8, 2024 07:59
zachdisc pushed a commit to zachdisc/iceberg that referenced this pull request Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestS3RestSigner is flaky

2 participants