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

Fix net ssh make target dir optional #8

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

martin-muller
Copy link

Our usage of this plugin has changed and we're now facing two issues:

1. SFTP connection using SSH fails

This is a downstream error of net-ssh, fixed simply by bumping the dependency as proposed here #7.

2. Upload to root fails

The upload action of this plugin requires a target_dir, this directory is then used together with the file name to create a path to upload. If we use an empty string the resulting path is /{file_name} which the underlying net-sftp fails to create:

/Users/martinmuller/.rbenv/versions/2.7.8/lib/ruby/gems/2.7.0/gems/net-sftp-4.0.0/lib/net/sftp/operations/upload.rb:321:in `on_open': \e[31m[!] Net::SFTP::StatusException open /apple-app-site-association (2, "no such file")\e[0m open /apple-app-site-association (2, "no such file") (Net::SFTP::StatusException)

This makes uploading files to root of the storage impossible, I'm making the target_dir arg optional and accommodating for the non-existence in the path creation. I'm happy to amend the tests as well if you accept this change.

@oklimberg
Copy link
Owner

Hi, I will have a look at this on the weekend. I appreciate this pull request.
Adding tests for the upload into the root directory of the user would be nice.

@martin-muller martin-muller reopened this Dec 11, 2024
@martin-muller
Copy link
Author

@oklimberg I've added tests and tweaked the dependencies a bit to get the tests running

@oklimberg
Copy link
Owner

Great. However, I’m going on vacation tomorrow and I’m not sure if I’ll be able to review the changes and release a new version beforehand. Therefore, it might not happen until the beginning of January, but I'll try my best.

@martin-muller
Copy link
Author

Great. However, I’m going on vacation tomorrow and I’m not sure if I’ll be able to review the changes and release a new version beforehand. Therefore, it might not happen until the beginning of January, but I'll try my best.

no worries, we're pinning my commit in the meantime, enjoy your vacation!

@martin-muller
Copy link
Author

@oklimberg were you able to review the proposed changes?

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.

2 participants