Sprint 6 work by jacobdadams · Pull Request #175 · agrc/palletjack

jacobdadams · 2025-12-30T00:14:24Z

replace pysftp with fabric (solves paramiko pin), closes Replace pysftp with paramiko #142
- Steve's PR replaced fabric with pure paramiko
use folder.add instead of content.add when uploading gdb for service updating
check for np.inf values, closes Check for np.inf in dataframes #121

codecov · 2025-12-30T00:28:06Z

Codecov Report

❌ Patch coverage is 96.82540% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.69%. Comparing base (f87f4c5) to head (44fbada).
⚠️ Report is 122 commits behind head on main.

Files with missing lines	Patch %	Lines
src/palletjack/extract.py	95.91%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #175      +/-   ##
==========================================
+ Coverage   94.52%   95.69%   +1.16%     
==========================================
  Files           7        7              
  Lines        1133     1254     +121     
  Branches      148      147       -1     
==========================================
+ Hits         1071     1200     +129     
+ Misses         52       44       -8     
  Partials       10       10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

steveoh

This all looks sound to me.

For fun and my own curiosity, gemini replaced the fabric bits with paramiko directly and it's straight forward for the SFTP transfers.

I accept that fabric has a more friendly api and is the paramiko recommendation. For this usage, paramiko doesn't seem to be too low level or advanced.

If there is a desire for faster build times and a leaner container memory wise, I think it's worth considering paramiko.

jacobdadams · 2025-12-30T17:55:43Z

You're welcome to spend the time to submit a commit that uses paramiko and updates/replaces all the tests. Otherwise, I'm sticking with fabric.

steveoh · 2025-12-30T18:13:38Z

You're welcome to spend the time to submit a commit that uses paramiko and updates/replaces all the tests. Otherwise, I'm sticking with fabric.

Thanks for the offer, but that isn't a priority for me right now. I approved this so unless you are going to wait for Scott, 🚢

jacobdadams · 2025-12-30T19:38:47Z

@stdavis, when you're back and dug out, I would appreciate your review of the folder.add changes; I cribbed them from dolly-carton and I think it was straight forward but just want to check. This is also evidence that you were right to want to move some of that into its own library.

steveoh · 2025-12-31T17:34:38Z

@copilot create a child pull request replacing the fabric dependency with paramiko.

Scope:

Fabric is currently only used for SFTP operations. Do not implement any general SSH command execution.
The current authentication does not use SSH keys (password/plain auth).

Implementation Details:

Follow all coding standards, formatting styles, and naming conventions found in the project.
Replace fabric in install_requires (or requirements files) with paramiko.
Important: Implement a context manager (using with or try/finally) to ensure the SFTP session and the underlying Transport are always closed, even if errors occur.
Map fabric exceptions to standard Python exceptions or paramiko specific ones where appropriate.
Refactor tests to use paramiko. Ensure you mock both paramiko.Transport and paramiko.SFTPClient correctly to avoid actual network calls.
Verify that no fabric imports remain.
Ensure all refactored tests pass.

Copilot · 2025-12-31T17:34:46Z

@steveoh I've opened a new pull request, #176, to work on those changes. Once the pull request is ready, I'll request review from you.

stdavis

LGTM 👍

Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

Added 7 new test cases to improve coverage: - Success path with proper cleanup - Connection failure handling - Authentication failure handling - SFTP client creation failure - Exception during operations - Integration tests with real context manager Coverage increased from 94% to 96% Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

- Fixed invalid escape sequences in regex patterns (extract.py lines 137, 195) Changed to raw strings (r"...") for proper regex syntax - Fixed FutureWarning for incompatible dtype assignment (load.py line 577) Changed from np.nan to pd.Series(dtype="object") for proper string/NaN handling All tests pass (369/370), coverage maintained at 96% Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

Changed from pd.Series(dtype="object") to pd.Series([np.nan] * len(df), dtype="object", index=df.index) to properly handle index alignment and maintain np.nan for compatibility with tests All tests pass (369/370), coverage maintained at 96% Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

- Added overwrite_existing_files boolean parameter to download_sftp_folder_contents (default=True) - Raises FileExistsError when overwrite_existing_files=False and file exists - Changed file counting to track downloaded files instead of directory count - Fixed path separator issues by using Path.as_posix() for cross-platform compatibility - Added 2 new tests for overwrite behavior - Updated all affected tests to handle new behavior All 371 tests pass, coverage maintained at 95% Co-authored-by: jacobdadams <38168030+jacobdadams@users.noreply.github.com>

Moved empty file list check inside first SFTP connection context to avoid opening second connection when no files to download All 371 tests pass, coverage maintained at 95% Co-authored-by: jacobdadams <38168030+jacobdadams@users.noreply.github.com>

jacobdadams · 2026-01-05T17:39:42Z

Ok, I've pulled in the paramiko branch, so if it still looks good to you guys we'll merge this into main (and hopefully kick off a new 6.0 release)

jacobdadams added 5 commits December 16, 2025 11:58

wip: replace pysftp

d3a8a9f

feat: replace pysftp with fabric, paramiko.

a278034

tests: new sftp methods

276d58f

fix: use folder.add instead of content.add to upload gdb

3e119ee

tests: consistent path to str formatting

240e500

jacobdadams added 3 commits December 29, 2025 17:52

chore: clean up pandas value being set on a copy warning

f7ac6ae

chore: check for np.inf

0eae06a

feat!: ftp breaking changes (empty)

954c25e

jacobdadams marked this pull request as ready for review December 30, 2025 17:08

jacobdadams requested review from stdavis and steveoh December 30, 2025 17:08

steveoh previously approved these changes Dec 30, 2025

View reviewed changes

jacobdadams changed the title ~~wip: sprint 6 work~~ Sprint 6 work Dec 30, 2025

jacobdadams mentioned this pull request Dec 30, 2025

Palletjack Updates agrc/planning-queue#274

Open

1 task

Copilot AI mentioned this pull request Dec 31, 2025

Replace fabric with paramiko for SFTP operations #176

Merged

30 tasks

stdavis previously approved these changes Jan 1, 2026

View reviewed changes

Copilot AI and others added 8 commits January 5, 2026 10:35

Replace fabric with paramiko for SFTP operations

e233ce5

Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

Refactor paramiko implementation to use context manager helper

08836b9

Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

Refactor test to avoid hardcoded list duplication

2dced8c

Co-authored-by: steveoh <325813+steveoh@users.noreply.github.com>

jacobdadams dismissed stale reviews from stdavis and steveoh via 44fbada January 5, 2026 17:35

jacobdadams requested review from stdavis and steveoh January 5, 2026 17:43

steveoh approved these changes Jan 5, 2026

View reviewed changes

jacobdadams merged commit eeea8f8 into main Jan 5, 2026
8 checks passed

jacobdadams deleted the dev branch January 5, 2026 18:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sprint 6 work#175

Sprint 6 work#175
jacobdadams merged 16 commits intomainfrom
dev

jacobdadams commented Dec 30, 2025 •

edited

Loading

Uh oh!

codecov bot commented Dec 30, 2025 •

edited

Loading

Uh oh!

steveoh left a comment

Uh oh!

jacobdadams commented Dec 30, 2025

Uh oh!

steveoh commented Dec 30, 2025

Uh oh!

jacobdadams commented Dec 30, 2025

Uh oh!

steveoh commented Dec 31, 2025

Uh oh!

Copilot AI commented Dec 31, 2025

Uh oh!

stdavis left a comment

Uh oh!

jacobdadams commented Jan 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jacobdadams commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

steveoh left a comment

Choose a reason for hiding this comment

Uh oh!

jacobdadams commented Dec 30, 2025

Uh oh!

steveoh commented Dec 30, 2025

Uh oh!

jacobdadams commented Dec 30, 2025

Uh oh!

steveoh commented Dec 31, 2025

Scope:

Implementation Details:

Uh oh!

Copilot AI commented Dec 31, 2025

Uh oh!

stdavis left a comment

Choose a reason for hiding this comment

Uh oh!

jacobdadams commented Jan 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jacobdadams commented Dec 30, 2025 •

edited

Loading

codecov bot commented Dec 30, 2025 •

edited

Loading