Skip to content

s3: fix multipart upload debug log#219

Merged
giacomo-alzetta-aiven merged 1 commit intoAiven-Open:mainfrom
gqmelo:gqmelo/fix-s3-multipart-log
Jan 14, 2026
Merged

s3: fix multipart upload debug log#219
giacomo-alzetta-aiven merged 1 commit intoAiven-Open:mainfrom
gqmelo:gqmelo/fix-s3-multipart-log

Conversation

@gqmelo
Copy link
Contributor

@gqmelo gqmelo commented Jan 14, 2026

About this change - What it does

The log would raise an exception whenever the total number of parts is unknown because it tried to format <?> as a number (introduced by #215).

Why this way

The issue could have been detected by the current tests if we weren't mocking the logger.
Pytest already offers better ways to capture the logs and assert expected logs, so use the caplog fixture instead.

Additionally I also switched the tests to use tmp_path fixture which is better than creating a regular temp file as it will be in dedicated dirs associated to test names.

The log would raise an exception whenever the total number of parts
is unknown.

The issue could have been detected by the current tests if we weren't
mocking the logger.
Pytest already offers better ways to capture the logs and assert
expected logs, so use the `caplog` fixture instead.

Additionally I also switched the tests to use `tmp_path` fixture which
is better than creating a regular temp file as it will be in dedicated
dirs associated to test names.
@gqmelo gqmelo force-pushed the gqmelo/fix-s3-multipart-log branch from 51a4c9d to e4bac81 Compare January 14, 2026 11:51
Copy link
Contributor

@giacomo-alzetta-aiven giacomo-alzetta-aiven left a comment

Choose a reason for hiding this comment

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

LGTM!

@giacomo-alzetta-aiven giacomo-alzetta-aiven merged commit 8455f6b into Aiven-Open:main Jan 14, 2026
7 checks passed
@gqmelo gqmelo deleted the gqmelo/fix-s3-multipart-log branch January 14, 2026 12:56
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.

2 participants