Skip to content

Filter out retry events for progress bar assertion on intg tests #5972

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

Closed
wants to merge 1 commit into from

Conversation

RanVaknin
Copy link
Contributor

Our S3 transfer manager integ test has a progress bar assertion that is intermittently failing due to an unaccounted retry message.

[ERROR] software.amazon.awssdk.transfer.s3.progress.LoggingTransferListenerTest.test_customTicksListener_successfulTransfer -- Time elapsed: 0.027 s <<< FAILURE!
org.opentest4j.AssertionFailedError:

expected: "|==== | 80.0%"
but was: "Retrying Request: DefaultSdkHttpFullRequest(httpMethod=PUT, protocol=http, host=localhost, port=42741, encodedPath=/bucket/key, headers=[amz-sdk-invocation-id, Content-encoding, Content-Length, Content-Type, Expect, User-Agent, x-amz-content-sha256, x-amz-decoded-content-length, x-amz-sdk-checksum-algorithm, x-amz-trailer], queryParameters=[partNumber, uploadId])"

We can filter out this "Retrying Request" log from the stream so that only the progression bar logs appear.

@RanVaknin RanVaknin requested a review from a team as a code owner March 18, 2025 21:07
@RanVaknin RanVaknin changed the title Filter out retry events for progress bar assertion Filter out retry events for progress bar assertion on intg tests Mar 18, 2025
@RanVaknin
Copy link
Contributor Author

Integ tests are passing

Copy link
Contributor

@L-Applin L-Applin left a comment

Choose a reason for hiding this comment

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

Do we know why the request was retrying in the first place?

@RanVaknin
Copy link
Contributor Author

The test in question tests the TransferListenerContext directly, and not uses the underlying transfer manager/ s3 client. Therefore I don’t think the log for the “Retrying Request” we are seeing is actually from this test.
The logger might get a shared scope somehow (maybe here?) and the error messages are getting polluted from other s3 tests that are running concurrently.

@RanVaknin RanVaknin added this pull request to the merge queue Mar 19, 2025
@joviegas
Copy link
Contributor

I think we donot require this fix after #5948

@RanVaknin RanVaknin removed this pull request from the merge queue due to a manual request Mar 19, 2025
@RanVaknin RanVaknin closed this Mar 19, 2025
@RanVaknin RanVaknin deleted the rvaknin/fix-integration-test-failures branch March 19, 2025 22:12
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