Skip to content

REFACTOR: Align sftp task path handling with the file task#70

Merged
Shivang Nagta (ShivangNagta) merged 9 commits into
patterninc:mainfrom
ShivangNagta:refactor/align-sftp-with-file
Jun 8, 2026
Merged

REFACTOR: Align sftp task path handling with the file task#70
Shivang Nagta (ShivangNagta) merged 9 commits into
patterninc:mainfrom
ShivangNagta:refactor/align-sftp-with-file

Conversation

@ShivangNagta

@ShivangNagta Shivang Nagta (ShivangNagta) commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to the SFTP task PR, aligning the sftp task a little more with the existing file
task after feedback by Divyanshu Tiwari (@divyanshu-tiwari)

Changes:
The tables below show what each role supports, and what changed

Field rename

Before After
remote_path path

Upload (sink) — path behavior

Scenario Before After
path is a full file path (/out/x.csv) Writes to that path Writes to that path (unchanged)
path is a directory (/out/) Auto-appended the source filename from context Errors - explicit filename needs to be given by template {{ context "CATERPILLAR_FILE_NAME_WRITE" }}

Download (source) — path patterns

Pattern Example Before After
Single file /out/a.csv Supported Supported (unchanged)
Single-level glob /out/*.csv, ?, […] Supported (path.Match) Supported (doublestar.Match)
Recursive glob ** /out/**/*.csv Single-level only - matched one directory deep, no recursion Supported — matches at any depth (doublestar)
Brace expansion /out/{a,b}/*.csv Not matched Supported - {a,b} expands to alternatives (doublestar)
Bare directory /out/ Expanded (all files in dir) Not supported - errors; use a glob (/out/*)
Missing single file /out/gone.csv Errors Errors (unchanged)
Glob with no match /out/*.json (base dir exists) Empty success Gives error (no files found)

On doublestar: the file task already globs with the
doublestar library - doublestar.Glob in its local reader and doublestar.Match (over the keys it
lists) in its S3 reader. doublestar extends Go's standard path.Match with
recursive ** (match across any number of directories) and {a,b} brace
expansion. The sftp source previously used pkg/sftp's Glob, which is
path.Match-based (single-level, no ** or {a,b}). This PR switches it to
the same doublestar.Match, so the sftp task now accepts the same glob syntax
as the file task.

Test:
Screenshot 2026-06-03 at 11 02 25 PM

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation and I have updated the documentation accordingly.
  • I have added tests to cover my changes.

Shivang Nagta added 4 commits June 3, 2026 17:42
The sftp task now writes to  as it is given, dropping the directory
auto-append and the isDirLike/cache helpers, so it matches the file
write task. To reuse the source file's name, template the path, e.g.
{{ context CATERPILLAR_FILE_NAME_WRITE }}. Unnessary comments are dropped.

BREAKING:  is renamed to , and a directory destination
no longer auto-appends the source filename - use explicit templating.
Align download globbing with the file task: a glob path is now matched with
doublestar (so  and  work) by walking the static base directory,
instead of pkg/sftp's single-level path.Match. A bare directory is no longer
auto-expanded — use a glob (e.g.  or ) to read its files.

No new dependency — doublestar is already used by the file task.
@ShivangNagta Shivang Nagta (ShivangNagta) marked this pull request as ready for review June 4, 2026 07:50
@ShivangNagta Shivang Nagta (ShivangNagta) requested a review from a team as a code owner June 4, 2026 07:50
Copilot AI review requested due to automatic review settings June 4, 2026 07:50

Copilot AI left a comment

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.

Pull request overview

This PR refactors the sftp pipeline task to align its path semantics and glob behavior with the existing file task: it renames remote_pathpath, removes implicit filename appending on upload, and upgrades download globbing to support doublestar patterns (**, {a,b}).

Changes:

  • Rename SFTP config field from remote_path to path across code + docs + example pipelines.
  • Upload (sink) now treats path as an explicit per-record destination (no directory auto-append of upstream filename).
  • Download (source) now matches globs using doublestar.Match by walking and matching remote paths, enabling ** recursion and brace expansion.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/pipelines/sftp/upload.yaml Updates example upload pipeline to use path and explicitly template CATERPILLAR_FILE_NAME_WRITE.
test/pipelines/sftp/s3_to_sftp.yaml Updates S3→SFTP example to use path and explicit filename templating.
test/pipelines/sftp/download.yaml Updates example download pipeline to use path for globbing.
README.md Updates supported-task description to reflect SFTP’s supported modes (upload/download).
internal/pkg/pipeline/task/sftp/sftp.go Renames config field to Path and updates comments accordingly.
internal/pkg/pipeline/task/sftp/README.md Documents new path behavior and doublestar glob support; updates examples.
internal/pkg/pipeline/task/sftp/operations.go Implements new upload semantics and switches download globbing to doublestar matching over walked remote paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread internal/pkg/pipeline/task/sftp/README.md Outdated
Comment on lines +116 to +120
var matches []string
walker := client.Walk(globBase(remotePath))
for walker.Step() {
if err := walker.Err(); err != nil {
return nil, fmt.Errorf(`walking %q: %w`, remotePath, err)
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

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.

please explore doublestar v4, it has some function which I think can be used here.

Comment thread internal/pkg/pipeline/task/sftp/operations.go Outdated
Comment thread internal/pkg/pipeline/task/sftp/operations.go Outdated
Comment thread internal/pkg/pipeline/task/sftp/operations.go Outdated
@ShivangNagta

Copy link
Copy Markdown
Contributor Author

please explore doublestar v4, it has some function which I think can be used here.

Some points regarding doublestarv4:

  1. Only Globbase is something i think i can completely replace with their splitpattern function, they serve the same logic.
  2. There have been a few changes from v1 to v4, they changed their library for filesystem access to io/fs. Also some renaming of functions like Glob to FIlepathglob, which would require some changes in other tasks like file task.
  3. For walking, there is globwalk, but it expects its first argument as fs.FS, and sftp does not really return the same File type, so it won't be directly compatible.

func (s *sftp) download(client *pkgsftp.Client, output chan<- *record.Record) error {

remotePath, err := s.RemotePath.Get(nil)
remotePath, err := s.Path.Get(nil)

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.

Suggested change
remotePath, err := s.Path.Get(nil)
path, err := s.Path.Get(nil)

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Comment thread internal/pkg/pipeline/task/sftp/operations.go
Comment thread internal/pkg/pipeline/task/sftp/operations.go
@ShivangNagta Shivang Nagta (ShivangNagta) merged commit 097b0fc into patterninc:main Jun 8, 2026
7 checks passed
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.

3 participants