Skip GCS folder-marker keys in GCSToSFTPOperator

[yuseok89]

GCSToSFTPOperator copies every name GCSHook.list() returns, including the trailing-/ folder markers a GCS listing contains. Here a marker does not just add a stray file. It fails the task.

_resolve_destination_path normalises the target with os.path.normpath, which strips the trailing slash, so folder/ targets the file path <dest>/folder. GCS lists lexicographically, so the marker is written first. folder/file.txt then needs <dest>/folder to be a directory, and SFTPHook.create_directory raises "... already exists and is a file".

The fix drops a trailing-/ key only when it is also a strict prefix of another key in the same listing, meaning a directory whose children are right there. An object that merely happens to end in / has no such overlap and is left alone.

Before / After

Listing: folder/ (a marker), folder/file.txt, and lonely/ (a real object whose name ends in /).

Source object	Before	After
folder/	written as a file at <dest>/folder	skipped
folder/file.txt	never transferred, task fails	<dest>/folder/file.txt
lonely/	written as a file at <dest>/lonely	unchanged by this PR

Writing folder/ as a file is what breaks folder/file.txt. The path its parent needs is already taken, and the task dies there.

Verified end to end against a real GCS bucket and an SFTP server: the failure reproduces on main and the same run succeeds with this change.

---

Was generative AI tooling used to co-author this PR?

• Yes (please specify the tool below)
  • Opus 5

---

• 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.

[aaron-y-chen]

Question about move_object=True: a skipped key never reaches _copy_single_object, so it is not deleted either.

• Run 1 - listing ["folder/", "folder/file.txt"]

  • folder/ is skipped, folder/file.txt is moved, the bucket keeps folder/.

• Run 2 - listing ["folder/"]

  • no overlap now, so it is kept and copied to <dest>/folder, the path run 1 created as a directory.

What is the intended outcome for run 2 here?

[yuseok89]

Thanks for working through that sequence. There was not one. The rule keeps any trailing-slash key it cannot prove is a marker, which came from the S3/GCS operators where that is always safe. On SFTP a path cannot be both a file and a directory, so that no longer holds. I reproduced it against a real bucket and SFTP server, and run 2 fails on every run from then on.

Pushed a follow-up commit that makes the intent explicit. A trailing-slash key is also skipped when its destination is already a directory, so run 2 becomes a no-op. It only fires where the write would have failed anyway, so nothing transferable is dropped.

[olegkachur-e]

Thank you for submitting the PR, addressing real problem.

I think the behaviour of "stmth/" -> "dst/smth"(file) is a side effect of the os.path.normpath() call, that was introduced fairly recently, and never was a legit case.

The "name/" should always mean a folder, so we should create a folder on dst or do no-op in any case.

I think it makes sense to make additional parameter in a style "mind empty dirs" with default "False" value and and handle that. And if we don't mind empty dir we'd probably do not delete with move object command(?).

Should also lead to least corner cases to handle.

WDYT? @yuseok89

[yuseok89]

Thanks, this makes sense. I checked it against a real SFTP server, and you're right that writing name/ as a file only comes from the normpath change.

This PR set out to stop folder markers from breaking the transfer, but I think it works better built around one rule, that an empty folder is never silently dropped, which is also where the S3 side landed. On SFTP that would mean the following.

• A trailing-slash key is a folder, so it creates that directory and is never written as a file.
• With move_object, the marker is deleted after its directory is created, like any other moved object.

With the rule in place, the logic this PR added to guard those corner cases should go away as well.

For the switch, I wonder if create_intermediate_dirs could serve the same purpose instead of a new parameter. When it is False, the markers would be skipped with a log line and left in the bucket. Settling the rule this way does mean the default differs from what you suggested. I'd also extend its docstring to cover markers. What do you think?

[potiuk]

Hello @yuseok89 - thank you for your contributions to Apache Airflow!

The Airflow community has introduced a limit of 5 open pull requests at a time for contributors without write access to the repository. You currently have 11 open pull requests, so - as a one-time step of introducing the limit - we closed the ones where maintainers have not engaged yet:

• Add deferred task resume flow to task lifecycle diagram #65323 - Add deferred task resume flow to task lifecycle diagram
• Add TaskGroup-level retries to retry a whole group as a unit #68139 - Add TaskGroup-level retries to retry a whole group as a unit
• Dedent task docstrings used as doc_md #71108 - Dedent task docstrings used as doc_md
• Fix task duration increasing forever after Dag run timeout skips it #71549 - Fix task duration increasing forever after Dag run timeout skips it
• Ignore task start_date and end_date on manual runs #71758 - Ignore task start_date and end_date on manual runs
• Make HttpSensor defer when response_check is provided #71922 - Make HttpSensor defer when response_check is provided
• Flag ui-field-behaviour drift between hooks and provider.yaml in static checks #72089 - Flag ui-field-behaviour drift between hooks and provider.yaml in static checks
• Skip GCS folder-marker keys in GCSToSFTPOperator #72638 - Skip GCS folder-marker keys in GCSToSFTPOperator
• i18n(ko): Add missing translations (Sep25) #73677 - i18n(ko): Add missing translations (Sep25)

These pull requests stay open because maintainers are already engaged in them - they count towards your limit:

• Show recent task states on the Dags list page #69619 - Show task state counts for the latest Dag run on the Dags list page
• UI: Add a time-window scope to the Dags list run state filter #70470 - UI: Add a time-window scope to the Dags list run state filter

This is not a judgement of you or of your changes. We never told contributors before that opening many pull requests at once was a problem, so there is nothing to feel bad about - and nothing is lost: your branches, commits and the review history stay where they are.

What we ask you to do is to make your first prioritization decision: choose which of the pull requests above matter most to you, and reopen them (up to 5 open at a time, including the ones still open) with the "Reopen pull request" button or gh pr reopen <PR_NUMBER> --repo apache/airflow. Reopen the ones you are ready to follow through - keep them rebased, respond to review comments and fix failing checks.

While your pull requests are waiting for review, the most valuable thing you can do is help in other ways - reviewing other contributors' pull requests, helping with issues, and taking part in the discussions on the devlist and Slack.

Why we introduced the limit, what it means for you and how to reopen or restore a pull request is explained in https://github.com/apache/airflow/blob/main/contributing-docs/32_open_pull_request_limit.rst.

---

Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting

[olegkachur-e]

I wonder if create_intermediate_dirs could serve the same purpose instead of a new parameter. When it is False, the markers would be skipped with a log line and left in the bucket. Settling the rule this way does mean the default differs from what you suggested. I'd also extend its docstring to cover markers. What do you think?

Yes, I agree, it's related and could suite for this case.

Also I'm unsure if should be treated as a bug fix or breaking changes alredy (in terms of the semver and release notes), maybe release manager will help to identify in the future.

P.S.
Looking forward to have it reopen :-)