Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CUMULUS-3890: Address sftp provider multi-part/fastGet concern #3809

Open
wants to merge 28 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
83fd2c8
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Nov 22, 2023
aa314ee
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Nov 28, 2023
ae30573
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Nov 30, 2023
e3de4fb
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Jan 12, 2024
f195570
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Jan 16, 2024
e91d941
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Jan 25, 2024
a9d773b
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Jan 30, 2024
ee10ed1
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Feb 6, 2024
c7a2d79
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Feb 15, 2024
c301ea3
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Feb 20, 2024
03b9822
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Mar 12, 2024
5e52bb0
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Mar 25, 2024
25d28c1
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Apr 22, 2024
994e877
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Apr 29, 2024
7554d51
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 May 11, 2024
0cdad44
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Jun 24, 2024
81cecdf
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Jul 10, 2024
1e773b7
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Jul 25, 2024
4c2e461
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Aug 9, 2024
fac41f7
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Aug 13, 2024
5a63c76
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Aug 14, 2024
de8f086
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Aug 15, 2024
926291f
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Aug 23, 2024
47fa0a3
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Aug 26, 2024
e9ee557
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Aug 27, 2024
ef1dacc
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Sep 19, 2024
67b6145
Merge branch 'master' of https://github.com/nasa/cumulus
Nnaga1 Sep 26, 2024
4fb4a1d
first commit
Nnaga1 Sep 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
does not have the Serverless v2 SSL certifications installed.

### Changed

- **CUMULUS-3890**
- Changed fastGet used in the sftp-client package to have 0 concurrency
- **CUMULUS-3725**
- Updated the default parameter group for `cumulus-rds-tf` to set `force_ssl`
to 0. This setting for the Aurora Serverless v2 database allows non-SSL
Expand Down
4 changes: 3 additions & 1 deletion packages/sftp-client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ export class SftpClient {

log.info(`Downloading ${remoteUrl} to ${localPath}`);

await this.sftp.fastGet(remotePath, localPath);
await this.sftp.fastGet(remotePath, localPath, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the ticket AC mentions fastGet use is justified/confirmed as acceptable, and an alternative implementation is made configurable, with fastGet disabled by default.

how would I even make an alternate implementation available? like an option for them to have concurrency if their server can handle it, is there some process.env or other var I can use, a little confused about that

Copy link
Contributor Author

@Nnaga1 Nnaga1 Sep 26, 2024

Choose a reason for hiding this comment

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

fastGet use is justified/confirmed as acceptable: the other SFTP alternative would be the simple get, which isnt as performant and requires a bit more configuration and parameters to work properly. I think fastGet can continued to be used with some way or check on what the concurrency should be. If DAACs can handle concurrency than that can be left alone or configured, and if they can't it can be set to 0 by default. I was reading something about read/write streams as an alternative so that may be an option, but I think this is one as well.

concurrency: 0,
});

log.info(`Finished downloading ${remoteUrl} to ${localPath}`);
}
Expand Down