Skip to content

Fix S3 output client option routing - #650

Open
chivector wants to merge 2 commits into
fabiocaccamo:mainfrom
chivector:fix/route-s3-output-options
Open

chivector wants to merge 2 commits into
fabiocaccamo:mainfrom
chivector:fix/route-s3-output-options

Conversation

@chivector

Copy link
Copy Markdown

Describe your changes

Fix S3 output with both default and explicit client options. On current main, to_json(filepath="s3://...") fails because the writer requires s3_options; supplying that keyword instead sends it to the JSON encoder and raises another TypeError.

Remove the transport-only keyword before encoding and normalize omitted/None options to an empty mapping at S3 dispatch, matching the input path. Serializer options such as indent and sort_keys still reach the serializer, and the caller's options are preserved. The change adds no dependency or public method.

Related issue

Fixes #649.

Validation

  • Final new regressions on unchanged Git base e910141: 13 errors across two test methods; fixed source: all 13 subcases pass.
  • Standard full coverage/unittest run on Windows / CPython 3.12.14: 992 tests passed, aggregate coverage 99%. After the submit hook added a postponed-annotations import to the new test only, the final 13 subcases were rerun successfully; production code is unchanged.
  • Independent real-boto3 localhost HTTP checks: original source six TypeErrors and zero uploads; fixed source six successful PUTs, verifying exact bytes, default/None/empty/custom configuration, JSON/YAML/TOML output, and existing success-path cleanup. The public API was also rerun from the final Git checkout.
  • All configured pinned pre-commit hooks pass, including Ruff 0.17.0 and mypy 2.4.0 (66 files in the hook). Separate repository-wide Ruff lint/format and mypy on all 65 package source files also pass.
  • git diff --check and independent agent review pass.

No live AWS operations or full Python/OS matrix were run. Existing credential-gated S3 tests did not exercise AWS; the localhost upload checks use an isolated environment and disposable test values. The initial full test attempt needed the missing orjson test dependency; the completed 992-test run includes it.

Checklist before requesting a review

  • I have performed a self-review of my code.
  • I have added tests for the proposed changes.
  • I have run the tests and there are no errors.

AI assistance: OpenAI Codex investigated, implemented and tested this user-authorized contribution, including an independent agent review. No separate human review or personal production incident is claimed.

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 output fails with both omitted and explicit s3_options

1 participant