Skip to content

Fix prefix handling in Amazon transfer operators - #73269

Open
yuseok89 wants to merge 1 commit into
apache:mainfrom
yuseok89:fix-s3-transfer-prefix-handling
Open

yuseok89 wants to merge 1 commit into
apache:mainfrom
yuseok89:fix-s3-transfer-prefix-handling

Conversation

@yuseok89

@yuseok89 yuseok89 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

This PR aligns filename prefix handling with the documented behavior of the Amazon FTP and SFTP transfer operators.

Previously, string prefixes were matched anywhere in a filename. This could select unintended files. Renaming also replaced every occurrence of the prefix, including occurrences later in the filename.

Documented behavior. Each operator's docstring already describes prefix semantics, not substring semantics:

:param s3_filenames: ... or a string prefix that all filenames must match.
:param ftp_filenames: ... prefix that replaces the s3 prefix.

The old implementation didn't match this. This PR makes the code match the docs, not the other way around.

The updated behavior:

  • Selects only filenames that start with the configured source prefix.
  • Replaces only the leading source prefix when a destination prefix is configured.
  • Applies consistently to FTP-to-S3, SFTP-to-S3, S3-to-FTP, and S3-to-SFTP transfers.
  • Adds regression tests covering both file selection and prefix replacement.

Example, with ftp_filenames="pre_" / s3_filenames="new_":

Filename Should match? Before After
pre_one.txt yes new_one.txt new_one.txt
xpre_two.txt no selected (bug) excluded
pre_again_pre_.txt yes new_again_new_.txt (corrupted mid-filename) new_again_pre_.txt

Users relying on the undocumented substring-matching behavior may observe different file selection after this change.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)
    • gpt-5.6

  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

@boring-cyborg boring-cyborg Bot added area:providers provider:amazon AWS/Amazon - related issues labels Sep 17, 2026
@yuseok89
yuseok89 marked this pull request as ready for review September 17, 2026 14:18
@yuseok89
yuseok89 requested a review from o-nikolas as a code owner September 17, 2026 14:18
@vincbeck

Copy link
Copy Markdown
Contributor

@ramitkataria @seanghaeli

else:
ftp_filename: str = self.ftp_filenames
files = [f for f in list_dir if ftp_filename in f]
files = [f for f in list_dir if f.startswith(ftp_prefix)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am not sure how people use the string form of *_filenames in practice, but this could affect a few groups without any error being raised: anyone filtering on a suffix like ".csv", S3 users with nested keys under s3_key, and FTP servers whose nlst returns path-qualified entries, since FTPToS3Operator currently passes the listed name straight to retrieve_file. In all of those cases the task would now succeed having transferred zero files.

What do you think about:

  • Match and replace on the basename in the FTP operator so startswith works regardless of nlst output shape
  • If you think the suffix use case is common, we could discuss whether selection should stay lenient while only the rename is fixed, since the rename corruption is the clearer bug

Depending on the approach, I think we should add a warning in the changelog so users are aware.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good points. I agree that the FTP operator should use the basename for matching and replacement while retaining the original path for retrieval. I also agree that changing the matching behavior immediately could surprise existing users and should be noted in the Amazon provider changelog.

The startswith change was intended to resolve the mismatch between the documented prefix semantics and the current substring matching.

Would it make sense to preserve substring matching for now, emit a deprecation warning for non-prefix matches, clarify the behavior for nested S3 keys, and switch to prefix-only matching after the deprecation period?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:amazon AWS/Amazon - related issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants