@buhtz I have compared the old and new code (which definitely adds value via documentation).

Please let me be the one to raise my hand here because

1. the argument names could be more self-explanatory, eg.
   mid -> initial_ssh_command_size
   r -> initial_size_change

2. the _MID_INITIAL constant could be more self-explanatory, eg.
   _MID_INITIAL -> _DEFAULT_INITIAL_SSH_COMMAND_SIZE

3. the function name could be more self-explanatory, eg.
   -> probeMaxSSHCommandSize()

4. the file name could be aligned together with a better function name but
   for now I would keep it since it was mentioned in some docs AFAIR.

5. I would stick to the old while loop instead of recursion as best practice (until recursion makes the code significantly shorter or easier to read)

6. the docstring should mention that the probing loop does bisect (halve) the size change for each new probing step for quickly getting a result

I know all my proposals are "cosmetics" but I am great fan of "literate programming" (goal: code is so easy to read that the documentation is rarely needed) and refactoring the code is a great chance for doing it now :-)

Moin,

I agree to most of your points and committed the modifications.

4. the file name could be aligned together with a better function name but
   for now I would keep it since it was mentioned in some docs AFAIR.

OK, I didn't touch it.

5. I would stick to the old while loop instead of recursion as best practice (until recursion makes the code significantly shorter or easier to read)

I kept the recursion. Asking for readability and understandability it is IMHO a matter of taste. Some like it traditional with loops and others like recursions.
But from a theoretical point of view using recursion is more "pythonic". And it reduces complexity of code (in terms of "cyclomatic complexity" or just the indentions in the code). And reducing complexity reduce the risk for errors and mistakes.
There could be also a philosophic discussion about comparing the human brain and CPU. But I would put that on our reason-to-have-a-beer-together-list. 🍻 😃

I always try to avoid loops, if-then-else blocks, etc. This is very easy with Python and for me feels more natural. E.g. the part where it is asked for errno. There I eliminated the whole else-block.

I am great fan of "literate programming" (goal: code is so easy to read that the documentation is rarely needed) and refactoring the code is a great chance for doing it now :-)

I'm totally into it here with you.

EDIT: I followed the PEP8 naming conventions which recommends to use camel case only for class names but underscores for function and method names. Because of that it is now prope_max_ssh_command_size() instead of propeMaxSshCommandSize().

I have a question about the wording.

Do we probe the command size or the length of the argument? Or is it the same?

Technically the tests are done with ssh user@host printf LONGRANDOMSTRING.
The result is len('printf LONGRANDOMSTRING'). That is an argument to the ssh command, right?
The ssh user@host is ignored, which would be a command?

EDIT: And being more pedantic. 😄 SSH is not the limiting factor here but the shell (on remote machine) itself set the limit. So what we do here is IMHO not probe the length of an SSH argument or command but the length of an argument/command on a remote shell using SSH. Depending on the answer to that some names could be modified.

Technically the tests are done with ssh user@host printf LONGRANDOMSTRING.
The result is len('printf LONGRANDOMSTRING').
That is an argument to the ssh command, right?

Yes, technically a long string is sent as argument,
but I think the underlying idea is: "How many bytes can I send (and receive) via SSH"
not only the argument length (which changes if the command has a different length and the send-buffer is the limit).

EDIT: And being more pedantic. smile SSH is not the limiting factor here but the shell (on remote machine) itself set the limit.

Agreed, the question is if we want a function name like probe_max_command_length_via_ssh_on_remote_shell()
or reduce the name to the use case ("ssh") like

probeMaxSSHCmdSize()

I chose

• probe: because the value cannot be queried ("get*") somewhere but uses a step-wise algorithm
• ssh: because it is only important when using ssh (no other remote protocol)
• command: because I think the purpose is to determine the maximum command (incl. arguments) [buffer] length we can sent via ssh (and man ssh also calls this an "command")
• size: Normally I use "size" for bytes and "length" for characters which may differ in case of encodings like UTF-8) but since the long random string consists only of ASCII chars I assumed we measure bytes here (even though this is false on non-UTF-8 platforms like Windows with UTF-16 or similar)

PS: I think the old names may be taken from the original algorithm published eg. here (where "argument length" and "command length" is also mixed-up): https://www.theeggeadventure.com/wikimedia/index.php/Ssh_argument_length

Thanks for your thoughts. So I understand we can just keep it as it is. Am I right? 😄

Thanks for your thoughts. So I understand we can just keep it as it is. Am I right? smile

yes, thanks for investing your time into that!

BTW: On the dev channel we should discuss if we use underscores or camel case for object names in the future...

EDIT:

• I added the theeggeadvanture-URL as reference to the original solution. @Germar can you confirm that your code is based on that?
• I reset the initial command size back to the original value because of the original solution stating that on solaris you can have such long commands. The algorithm only works correct if the initial size does cause a "too long" error in the first place. Because of that it is better to have that value more high than lower.
• Fixed a bug with argparse.

• I added the theeggeadvanture-URL as reference to the original solution. @Germar can you confirm that your code is based on that?

That looks familiar. I probably based my code on this.