OpenBagTwo / EnderChest

syncing and linking for all your Minecraft instances
https://openbagtwo.github.io/EnderChest/
GNU General Public License v3.0
3 stars 2 forks source link

SFTP Sync Support #85

Closed OpenBagTwo closed 1 year ago

OpenBagTwo commented 1 year ago

Summary

Addresses #42 by implementing pure-Python syncing via SFTP and Paramiko

List of Changes

Tech Debt and Other Concerns

To Do

Validation Performed

PR Type

Checklist:

OpenBagTwo commented 1 year ago

I can't believe I forgot I'd already found (and then just failed to apply) the solution to the whac-a-bug...

OpenBagTwo commented 1 year ago

Well this is interesting... it looks like -p enderchest.test.plugin is completely messing up the coverage report...

For now I appear to have things working by generating the report manually:

coverage run --source enderchest -m pytest --use-local-ssh
coverage report -m

but I need to either find an existing bug report (and hopefully a workaround) or file a bug report.

OpenBagTwo commented 1 year ago

Yeah, this appears to be a known pytest-cov limitation: https://github.com/pytest-dev/pytest-cov/issues/587

OpenBagTwo commented 1 year ago

I'm calling it with regard to unit tests. Even though overall coverage is only up two percentage points (85%), coverage within the sync subpackage is looking good, with the stuff that's not tested (and not "no cover" pragma'd) being stuff that should be tested, but isn't currently worth the level of effort.

OpenBagTwo commented 1 year ago

Trying this for real, some observations:

  1. Due to SFTP having to individually scan every single sub-directory on the remote, computing a diff is extremely slow (for my 22GB EnderChest :grin: )
  2. Because of the way that dry runs work, it has to perform the diff twice for a typical sync operation (it also prompts you twice for the password). That's not wrong, per se (the diff could change in the pause interval, especially if it's asking for a manual confirmation) but it isn't a great user experience.
OpenBagTwo commented 1 year ago

I performed an enderchest open and used the -v flag to track the progress, but cancelled something like halfway through ('cuz recursing through 22G of Minecraft files is sloooowwww).

I also ran an enderchest close -v --dry-run which produced the expected diff, so I'm feeling good enough about this to merge.

For the release candidate, I'll set up a smaller test chest. And, of course, if anyone reading this dailies Windows and wants to try @dev between now and the RC, I'd definitely love feedback!