fix(dagster-aws): handle s3_prefix=None in PickledObjectS3IOManager

[Vamsi-klu]

Reviewers Requested: @smackesey @gibsondan — domain experts for dagster-aws (top committers to this package)

Note on CI: Buildkite builds are blocked pending maintainer approval (standard for fork PRs). Tests pass locally — see Test Results below.

Summary & Motivation

Fixes #33490

PickledObjectS3IOManager accepts s3_prefix=None in its constructor signature (s3_prefix: str | None = None), but it was passed directly to boto3's list_objects(Prefix=None), which raises botocore.exceptions.ParamValidationError because boto3 strictly requires the Prefix parameter to be a string type.

This affects users who:

• Use the legacy s3_pickle_io_manager resource factory (where s3_prefix defaults to None if not configured)
• Explicitly pass s3_prefix=None to PickledObjectS3IOManager

Root Cause

In PickledObjectS3IOManager.__init__, line 35 directly passes the unchecked s3_prefix value to boto3:

self.s3.list_objects(Bucket=s3_bucket, Prefix=s3_prefix, MaxKeys=1)

When s3_prefix=None, boto3 raises:

botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter Prefix, value: None, type: <class 'NoneType'>, valid types: <class 'str'>

Fix

Added s3_prefix = s3_prefix or "" normalization after the check.opt_str_param call and before the boto3 call. An empty string is the correct boto3 equivalent of "no prefix" and is also falsy, so the existing base_path = UPath(s3_prefix) if s3_prefix else None logic continues to work correctly.

Test Plan

Test	Status
test_pickled_object_s3_io_manager_none_and_empty_prefix (parametrized: None, "")	PASS
test_pickled_object_s3_io_manager_with_valid_prefix	PASS
Full test_io_manager.py suite (7 tests)	PASS
make ruff	PASS

Test Results

7 passed in 2.23s  (dagster_aws_tests/s3_tests/test_io_manager.py)

Changelog

PickledObjectS3IOManager no longer raises ParamValidationError when s3_prefix=None.

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

Fixes a ParamValidationError in PickledObjectS3IOManager where passing s3_prefix=None caused boto3 to reject the Prefix parameter in list_objects. The fix normalizes None (and empty string) to "" immediately before the boto3 call.

• io_manager.py: Adds a single s3_prefix = s3_prefix or "" line after the type-check and before the list_objects call; the subsequent UPath(s3_prefix) if s3_prefix else None guard still works correctly since "" is falsy.
• test_io_manager.py: Adds a parametrized regression test covering both None and "" inputs, plus a sanity-check test for a non-empty prefix.

Confidence Score: 5/5

This PR is safe to merge — it makes a minimal, targeted fix to a well-understood bug with no behavioral changes for callers who already pass a valid non-empty prefix.

The change is a single-line normalization in the constructor that only affects the None/empty-string code path, which was previously broken entirely. The fix is correct: or "" converts both falsy values to an empty string accepted by boto3, and the existing if s3_prefix guard below it still correctly sets base_path=None in both cases. The regression tests directly cover the broken inputs and the happy path.

No files require special attention.

Important Files Changed

Filename	Overview
python_modules/libraries/dagster-aws/dagster_aws/s3/io_manager.py	Adds s3_prefix = s3_prefix or "" normalization before the boto3 call so that None is converted to an empty string, fixing a ParamValidationError when no prefix is configured.
python_modules/libraries/dagster-aws/dagster_aws_tests/s3_tests/test_io_manager.py	Adds two new tests: a parametrized regression test for None/"" prefix and a sanity-check test for a valid prefix. The boto3 client is imported inline rather than using the existing construct_s3_client helper, but this is a minor style nit.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["PickledObjectS3IOManager.__init__(s3_prefix)"] --> B["check.opt_str_param(s3_prefix)"]
    B --> C{"s3_prefix or ''"}
    C -- "None → ''" --> D["s3_prefix = ''"]
    C -- "'' → ''" --> D
    C -- "'my-prefix' → 'my-prefix'" --> E["s3_prefix = 'my-prefix'"]
    D --> F["s3.list_objects(Prefix='')"]
    E --> G["s3.list_objects(Prefix='my-prefix')"]
    F --> H{"s3_prefix truthy?"}
    G --> H
    H -- "No (empty string)" --> I["base_path = None"]
    H -- "Yes" --> J["base_path = UPath(s3_prefix)"]
    I --> K["super().__init__(base_path=None)"]
    J --> K

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["PickledObjectS3IOManager.__init__(s3_prefix)"] --> B["check.opt_str_param(s3_prefix)"]
    B --> C{"s3_prefix or ''"}
    C -- "None → ''" --> D["s3_prefix = ''"]
    C -- "'' → ''" --> D
    C -- "'my-prefix' → 'my-prefix'" --> E["s3_prefix = 'my-prefix'"]
    D --> F["s3.list_objects(Prefix='')"]
    E --> G["s3.list_objects(Prefix='my-prefix')"]
    F --> H{"s3_prefix truthy?"}
    G --> H
    H -- "No (empty string)" --> I["base_path = None"]
    H -- "Yes" --> J["base_path = UPath(s3_prefix)"]
    I --> K["super().__init__(base_path=None)"]
    J --> K

Loading

_{Reviews (2): Last reviewed commit: "fix(dagster-aws): handle s3_prefix=None ..." | Re-trigger Greptile}

[smackesey]

Good fix, thank you!

[smackesey]

Merge activity

• Mar 16, 10:34 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Mar 16, 10:34 AM UTC: Graphite couldn't merge this PR because it failed for an unknown reason (Fast-forward merges are not supported for forked repositories. Please create a branch in the target repository in order to merge).
• Mar 16, 7:29 PM UTC: A user started a stack merge that includes this pull request via Graphite.
• Mar 16, 7:30 PM UTC: Graphite couldn't merge this PR because it failed for an unknown reason (Fast-forward merges are not supported for forked repositories. Please create a branch in the target repository in order to merge).

[Vamsi-klu]

@smackesey are there any actions on my end to help this PR merged?

[smackesey]

whoops, forgot to hit the button. Merged!

[Vamsi-klu]

@smackesey thanks for approving this one! It's approved but the merge looks gated on CI not having run for the fork PR. Could you trigger CI / merge when you get a chance, or let me know if anything else is needed? Appreciate it.

[Vamsi-klu]

@smackesey following up because this PR is still open and still approved on the same head (321d040).

The Graphite stack-merge attempts on Mar 16 failed with "Fast-forward merges are not supported for forked repositories", so the "Merged!" comment did not actually merge the PR. Could you merge it via the GitHub UI, or otherwise from a target-repo branch, when you get a chance?

I have not rebased or changed the branch so the approved head stays intact. Thanks.