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

Checksum command fails if remote doesn't support uname operation #16

Open
pratheekrebala opened this issue Feb 3, 2022 · 1 comment

Comments

@pratheekrebala
Copy link

pratheekrebala commented Feb 3, 2022

When I try to fs.checksum(path) on a server that does not permit a uname command, I get a generic Channel Open Error: Session request failed. Looking through the debug logs, it seems like the issue is caused by the logic in the _get_system:

https://github.com/fsspec/sshfs/blob/3c10c1bfff44f111926d763a54343726832e2d42/sshfs/spec.py#L295:L300

The server I am working with does support the md5sum and sha1sum commands so the _checksum method as written should work but the actual command never triggers because it errors before that.

Potential Solutions:

  • Provide ability to pass checksum commands to the _checksum() method, this would allow me to specify a known command (so the _get_system() check can be bypassed). Ideally, being able to provide both the remote and the local checksum commands would be even better in the case of a Darwin system speaking with a Linux server.
  • Include error handling logic for _get_system() to provide a more detailed error message.

If I can get some guidance on the preferred approach, and if contributions are welcome. I'm happy to submit a PR.

Thank you!
Pratheek

@efiop
Copy link
Member

efiop commented Apr 6, 2022

Hi @pratheekrebala !

fs.checksum implementation probably has too much influence from our use in dvc and likely shouldn't call commands at all and rather just use the info() result, similar to what localfs does. If you need to run md5sum or sha1sum command, it is probably best to just run it yourself, as running commands is mostly out of scope for sshfs itself. I think we'll need to adjust fs.checksum in the future.

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

No branches or pull requests

2 participants