-
Notifications
You must be signed in to change notification settings - Fork 599
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
SCP : limit the used bandwidth #208
Conversation
Hi! Thanks for the PR. Unfortunately, Travis claims that one test is failing. Can you have a look and fix it? |
Hi, |
@@ -28,18 +28,23 @@ | |||
extends AbstractFileTransfer | |||
implements FileTransfer { | |||
|
|||
/** Default bandwidth limit for SCP transfert in Kbit/s (-1 means unlimited) */ |
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.
typo in comment (transfert). Also is it kilobit, or kilobyte? The difference is a factor 8.
Sorry for the typo. It's kilobit/s, according to the scp man page. |
|
||
private final char a; | ||
private String v; |
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.
Why the extra 'v' here? It is not used currently. The LIMIT
seems to use it, but as it is an enum it is always the empty string.
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.
Some scp options needs a key/value structure. The value is initialized with an empty string but can be set through the helper class SCPArguments. This the way I have used to set the value of the LIMIT arg.
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.
Ahh but this is even more evil, as this is an enum, setting the value for 1 process means that you're setting it (inadvertently) for all processes. Because there is only 1 instance of the same enum key in a JVM. So please refactor that and remove the setValue(..)
Thanks! Looks good now I think. Will do some testing on it, and then merge it in next week. |
Nice, thank you. |
SCP : limit the used bandwidth
Merged, thx! |
Hello,
I have a strong requirement to limit usage of bandwidth at application level when I upload files to a remote server.
According to the scp man page, I have added a '-l XXX' arg to the SCPEngine class.
Then, to keep the fluent aspect of your API, this new arg can (optionally) be set as below:
A test case is provided into SCPFileTransferTest for upload and download features.
Thanks.