Skip to content

fix(s3): correct S3 block size error messages to match checks - #984

Closed
Jah-yee wants to merge 1 commit into
pyathena-dev:masterfrom
Jah-yee:fix/block-size-error-messages
Closed

Jah-yee wants to merge 1 commit into
pyathena-dev:masterfrom
Jah-yee:fix/block-size-error-messages

Conversation

@Jah-yee

@Jah-yee Jah-yee commented Oct 3, 2026

Copy link
Copy Markdown

Summary

Three S3 block size error messages did not match the actual validation logic:

  1. s3.py:1278 & s3_async.py:284: The check uses block_size < MIN_PART_SIZE or > MAX_PART_SIZE (inclusive bounds), but the message said "greater than 5MiB and less than 5GiB" (exclusive). Updated to "at least 5 MiB and at most 5 GiB" to match the actual inclusive bounds.

  2. s3.py:2325: The message showed the byte count (5242880) with an "MB" suffix, which is confusing. Updated to "5 MiB (5242880 bytes)" to make the units clear.

Changes

  • pyathena/filesystem/s3.py: Fix two ValueError messages
  • pyathena/filesystem/s3_async.py: Fix one ValueError message

Reproduction

from unittest.mock import MagicMock
from pyathena.filesystem.s3 import S3FileSystem

fs = S3FileSystem(connection=MagicMock(), skip_instance_cache=True)
fs.open("s3://bucket/key", "wb", block_size=1024)
# Before: ValueError: Block size must be >= 5242880MB.
# After:  ValueError: Block size must be >= 5 MiB (5242880 bytes).

Fixes #926.

- s3.py: Fix message to say 'at least 5 MiB (5242880 bytes)' instead of
  incorrectly showing byte count with MB suffix
- s3.py & s3_async.py: Fix 'greater than' to 'at least' and 'less than' to
  'at most' since the checks use inclusive bounds [<, >]

Fixes pyathena-dev#926.
@laughingman7743

Copy link
Copy Markdown
Member

Thank you for the fix, @Jah-yee!

#926 was already being fixed together with the related S3File write validation (#976 and #952), which changes the same checks and messages, in #992. So I'm closing this PR in favor of that one. Thanks again for taking the time to look into it.

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.

S3 filesystem block size error messages do not match their checks

2 participants