Implement CopyFromRemote for io.Reader filesink #12

enmand · 2019-01-23T02:09:55Z

This PR introduces CopyFromRemote (from #1) which returns an io.Reader with the file contents, the remote file permissions (as an int) and an error if it occurred.

Closes #1

enmand · 2019-01-23T02:11:19Z

client.go

+	return errors.New("no scp cmd found")
+}
+
+func pipes(s *ssh.Session) (io.Reader, io.Reader, io.WriteCloser, error) {


pipes probably doesn't need to return stderr, but maybe it would be useful?

bramvdbogaerde · 2019-01-23T19:15:30Z

This looks very interesting.
I will try to review this by the end of the week.

bramvdbogaerde

About command.go:

I very much like the idea to represent the command itself as a struct.
However I am unsure about the format of the permissions.
I think they should be in octal base right?

Futhermore, converting the integers to a string is a bit redundant as the Sprintf function would happily take integers by using "%d", the same applies to octal representation by using "%o", however it is possible that you will need to zero pad that one on the left, so that 0777 is really converted to 0777.

bramvdbogaerde

About the other file:

This is quite a lot of code.
I will take a look at it later this week.

enmand · 2019-01-24T22:29:08Z

However I am unsure about the format of the permissions.
I think they should be in octal base right?

Futhermore, converting the integers to a string is a bit redundant as the Sprintf function would happily take integers by using "%d", the same applies to octal representation by using "%o", however it is possible that you will need to zero pad that one on the left, so that 0777 is really converted to 0777.

Ah yeah -- good catch! I didn't use the MarshalText() so I didn't see that. Maybe I should add some tests too! 😄

enmand · 2019-01-24T22:30:06Z

client.go

+
+	var i int
+	for {
+		i = i + 1


This was debug code that can be removed now, along with the var i int

… error) Add a CopyFromRemote which returns an io.Reader with the contents of the remote file

enmand · 2019-02-03T18:58:31Z

Hello! I've cleaned up the code to deal with permissions to use os.FileMode and properly use the octal representations, and added some tests for the Command Marshal and Unmarshal

- Change size to uint64

bramvdbogaerde · 2019-02-09T20:33:02Z

I am sorry for the late response.
Many thanks for your contributions.

I will take a look at it next week.

clarkezone · 2019-03-24T23:26:28Z

any chance this will get merged soon?

bramvdbogaerde · 2019-03-25T06:51:41Z

Yes, @clarkezone, I will try to make time to look at the changes this week.

hungarianguy · 2019-06-05T23:41:22Z

@bramvdbogaerde @clarkezone is this something you plan to merge sometime soon?

bramvdbogaerde · 2019-06-06T11:44:27Z

Indeed, it has been a while since this pull request was first created.

I think the idea is great, I do like the implementation but I have some small issues with some parts of the code, currently I don't have much time to carefully review the code and make some changes where necessary. I will try to do that by the end of the month.

clarkezone · 2019-06-27T22:26:22Z

Any updates on this?

elie-g · 2019-08-02T15:56:10Z

@bramvdbogaerde If you don't have time to maintain the project maybe you could add a new maintainer. If you don't know anyone interested in becoming the new maintainer, you can create an issue and ask who is interested. See this issue

bramvdbogaerde · 2019-08-31T14:36:38Z

I am sorry for my late response @DrunkenPoney, this project is certainly not dead, as can be demonstrated by my recent commits.

I plan to work on it more frequently now, and hope to have the suggested features added soon.

ceprateek · 2019-09-30T20:48:56Z

when is this change going to be merged ? This is a handy functionality which is needed.

cpanato · 2019-11-05T14:17:31Z

Hello Any plans to get this merged?

ChadTaljaardt · 2020-02-04T20:53:40Z

Is this PR dead?

bramvdbogaerde · 2020-04-28T19:18:39Z

I am sorry to keep you all waiting for so long.

I will try to review this pull request somewhere in the next few weeks and make
some adjustments to keep up with the current code base if necessary.

tupyy · 2020-05-14T11:48:09Z

Hello,

I'm playing around with this improvement because I need to write a lambda with is reading from a remote file.
It is working fine but the final size of the copied file is one byte bigger than the original size.
Could you explained me why please?

Thanks a lot.

bramvdbogaerde · 2021-05-23T20:16:34Z

At last, this was merged in PR #30, that PR has taken some inspiration from this PR, so many thanks for the changes here too.

Add Command struct for parsing SCP commands

c7d3514

enmand commented Jan 23, 2019

View reviewed changes

enmand force-pushed the improvements/filesink branch 2 times, most recently from 2cdceae to 8f1c139 Compare January 23, 2019 02:27

bramvdbogaerde self-requested a review January 23, 2019 19:25

bramvdbogaerde requested changes Jan 23, 2019

View reviewed changes

bramvdbogaerde reviewed Jan 23, 2019

View reviewed changes

enmand commented Jan 24, 2019

View reviewed changes

client.go Outdated

var i int

for {

i = i + 1

Copy link

Author

enmand Jan 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was debug code that can be removed now, along with the var i int

enmand added 2 commits February 3, 2019 14:57

Add CopyFromRemote(path string) (file io.Reader, permissions int, err…

dfa5c0b

… error) Add a CopyFromRemote which returns an io.Reader with the contents of the remote file

Update struct for parsing command to better deal with permissions

1c253fb

enmand force-pushed the improvements/filesink branch from 0a77f33 to 05d2de4 Compare February 3, 2019 18:57

enmand added 2 commits February 3, 2019 16:12

Add NewCommand for parsing and creating command

11aa7e4

- Change size to uint64

Return os.FileMode from Command

4f71c06

enmand force-pushed the improvements/filesink branch from 05d2de4 to 525c0b1 Compare February 3, 2019 20:13

Add tests for SCP Command struct

f1c4db5

enmand force-pushed the improvements/filesink branch 2 times, most recently from b6becfb to dcca06c Compare February 3, 2019 20:36

Use Command.MarshalText for Copy scp command marshalling

cda28e9

enmand force-pushed the improvements/filesink branch from dcca06c to cda28e9 Compare February 3, 2019 20:39

bramvdbogaerde mentioned this pull request Apr 28, 2020

Is it possible to copy from a remote path? #27

Closed

Teajo mentioned this pull request May 20, 2020

Small try on copyFromRemote feature #30

Merged

bramvdbogaerde mentioned this pull request Jun 23, 2020

downloadFile function missing #31

Closed

bramvdbogaerde closed this May 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement CopyFromRemote for io.Reader filesink #12

Implement CopyFromRemote for io.Reader filesink #12

enmand commented Jan 23, 2019

enmand Jan 23, 2019

bramvdbogaerde commented Jan 23, 2019

bramvdbogaerde left a comment •

edited

Loading

bramvdbogaerde left a comment •

edited

Loading

enmand commented Jan 24, 2019

enmand Jan 24, 2019

enmand commented Feb 3, 2019

bramvdbogaerde commented Feb 9, 2019

clarkezone commented Mar 24, 2019

bramvdbogaerde commented Mar 25, 2019

hungarianguy commented Jun 5, 2019

bramvdbogaerde commented Jun 6, 2019 •

edited

Loading

clarkezone commented Jun 27, 2019

elie-g commented Aug 2, 2019 •

edited

Loading

bramvdbogaerde commented Aug 31, 2019

ceprateek commented Sep 30, 2019

cpanato commented Nov 5, 2019

ChadTaljaardt commented Feb 4, 2020

bramvdbogaerde commented Apr 28, 2020

tupyy commented May 14, 2020

bramvdbogaerde commented May 23, 2021

Implement CopyFromRemote for io.Reader filesink #12

Implement CopyFromRemote for io.Reader filesink #12

Conversation

enmand commented Jan 23, 2019

enmand Jan 23, 2019

Choose a reason for hiding this comment

bramvdbogaerde commented Jan 23, 2019

bramvdbogaerde left a comment • edited Loading

Choose a reason for hiding this comment

bramvdbogaerde left a comment • edited Loading

Choose a reason for hiding this comment

enmand commented Jan 24, 2019

enmand Jan 24, 2019

Choose a reason for hiding this comment

enmand commented Feb 3, 2019

bramvdbogaerde commented Feb 9, 2019

clarkezone commented Mar 24, 2019

bramvdbogaerde commented Mar 25, 2019

hungarianguy commented Jun 5, 2019

bramvdbogaerde commented Jun 6, 2019 • edited Loading

clarkezone commented Jun 27, 2019

elie-g commented Aug 2, 2019 • edited Loading

bramvdbogaerde commented Aug 31, 2019

ceprateek commented Sep 30, 2019

cpanato commented Nov 5, 2019

ChadTaljaardt commented Feb 4, 2020

bramvdbogaerde commented Apr 28, 2020

tupyy commented May 14, 2020

bramvdbogaerde commented May 23, 2021

bramvdbogaerde left a comment •

edited

Loading

bramvdbogaerde left a comment •

edited

Loading

bramvdbogaerde commented Jun 6, 2019 •

edited

Loading

elie-g commented Aug 2, 2019 •

edited

Loading