-
Notifications
You must be signed in to change notification settings - Fork 83
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
feature: backend sftp #68
Conversation
fix: typo in filesystem skeeker -> seeker
To test the sftp storage backend: Run Then replace TestSFTPTruth with this code: func TestSFTPTruth(t *testing.T) {
cli, err := InitializeSFTPBackend(SFTPConfig{
CacheRoot: "/upload",
Username: "foo",
Auth: SSHAuth{
Password: "pass",
Method: SSHAuthMethodPassword,
},
Host: "localhost",
Port: "22",
}, true)
if err != nil {
t.Fatal(err)
}
content := "Hello world4"
// PUT TEST
file, _ := os.Create("test")
_, _ = file.Write([]byte(content))
_, _ = file.Seek(0, 0)
err = cli.Put("test3.t", file)
if err != nil {
t.Fatal(err)
}
_ = file.Close()
// GET TEST
readCloser, err := cli.Get("test3.t")
if err != nil {
t.Fatal(err)
}
b, _ := ioutil.ReadAll(readCloser)
if !bytes.Equal(b, []byte(content)) {
t.Fatal(string(b), "!=", content)
}
_ = os.Remove("test")
} |
Missing test with a ftp server (not a sftp) don't merge right now :) |
@alexisvisco Good work so far, thanks a lot. Ping me when you're done. |
@kakkoyun Hey, I think I should do another pull request for the FTP part because the library used does not provide any ftp support, SFTP is a protocol to send and retrieve files over ssh. Maybe you should look for an FTP library and tell me what you want. |
You can merge 🙆♂ |
@alexisvisco Thanks a lot for the work. I will thoroughly review it, please be patient, it won't take long. In the meantime, feel free to create another PR for FTP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First of all, thanks a lot for your contributions. Quality of code is great.
However, we need to add automated tests and documentation to accept it.
Especially we need examples, please check what we already have.
I have also pointed out some minor issues. Please take a look at those as well.
@alexisvisco Have you made any progress? Do you need help? Just ping me if you need anything. |
Hi I have done the test and add a docker-compose service. Unfortunately the continuous integration test does not work and I don't really have time to dive into your drone configuration. Could you please test manually the sftp config with a drone test ? I have no more time to doing this PR and it's almost finished, just test the config and add details in the https://github.com/meltwater/drone-cache/blob/master/DOCS.md file. Sorry for the inconvenience. |
@alexisvisco Thank you very much @alexisvisco, it's all good. I can take it from here. Good work. To be able to push to this branch directly, could you give me access on your forked repo? |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is green now. Thanks a lot, @alexisvisco.
I have discovered a couple of issues; for those, I'll send a follow-up PR.
This is good to go for now.
Adding the sftp as backend for the cache storage.
Relation with issue #60
todo: