Consider forking Paramiko #275

Closed
bitprophet opened this Issue Aug 19, 2011 · 13 comments

Projects

None yet

3 participants

@bitprophet
Member

Description

General ticket to help track other issues that rely on us patching Paramiko in order to get Important Stuff done.


There is history re: Fabric + Paramiko that has caused us to lose confidence in Paramiko's packaging practices. It's alluded to in #86, but the gist was that Paramiko 1.7.4 was nuked from PyPI entirely after 1.7.5 released (a huge no-no, for starters) and 1.7.5 included an intermittent but highly damaging bug.

Combined with a nil response from Paramiko's maintainer over a months-long period, this forced us to bundle Paramiko 1.7.4 into our source tree until Paramiko 1.7.6 was released much later.


Whether we publicly fork (after confronting Robey about important patches not getting integrated) and host the fork on pypi (which could lead to us feeling responsible for other, non-Fabric-related patches, if we show up online as "the Paramiko like library that integrates patches quicker"), or just do a Github-type unofficial fork and refer to it in our setup.py with special hooks (which feels less clean and might lead to packaging problems for distro packages) is something to decide later.

See related tickets below for the list. Note that even "closed" tickets may still be important and were just closed due to a workaround.


Originally submitted by Jeff Forcier (bitprophet) on 2011-01-14 at 02:33pm EST

Relations

  • Related to #228: PyCrypto >=2.1 has issues with pip
  • Related to #214: Potential Paramiko forking bug
  • Related to #185: Get better SSH testing infrastructure
  • Related to #85: Prompts for "passphrase" instead of "password" in some situations
  • Related to #72: SSH key forwarding
  • Related to #38: Implement tunnelling
  • Related to #217: Allow file handling methods (put/get/etc) to handle file-like objects and/or StringIO
  • Related to #19: Implement parallelism/thread-safety
@bitprophet bitprophet was assigned Aug 19, 2011
@bitprophet
Member

Jeff Forcier (bitprophet) posted:


Doesn't have its own ticket, but another area to potentially tweak is how Paramiko handles keys & agents -- apparently regular ssh, when given -i, looks in the SSH-agent for a matching key and will use that in preference to loading up the on-disk file.

The benefit of this approach is that you can force the SSH mechanisms to select a specific key instead of just trying all keys in the agent -- which is problematic with many keys as too many failed login attempts will usually cause lockout on the remote end. Furthermore, it allows you to have a key safely stored in an agent instead of requiring the passphrase to be entered again via plaintext at the time Paramiko/Fabric runs.

This isn't a terribly high priority, but it would be a nice-to-have re: making Paramiko/Fabric's default behavior be closer to that of ssh.


on 2011-08-16 at 04:31pm EDT

@bitprophet
Member

#19 seems to require us to do this now, since we need an additional patch on top of Paramiko 1.7.7.1 to prevent problems with large parallel runs.

It will change our dependencies, which sucks, but with luck it will not affect too many folks:

  • Package managers should be able to account for the change the next time they release a Fabric package, especially if we host on PyPI (though they should be able to handle a Github tarball too, I know at least some packagers do that for Fab itself.)
  • Other users should again not notice a problem with either type of hosting, because even easy_install can install from a Github tarball URL just fine, at least in my testing.

There is another angle, namely that 1.7.7.1 upped the PyCrypto dependency to 2.1+. This affects the install directions added in #228: Fabric 1.3+ will no longer have "PyCrypto 2.0.1 + pip <0.8.1 + Python 2.5" as a potential option. Almost a good thing; it simplifies those instructions to just "If you are installing via pip, you must have pip >=0.8.1."

Especially given that Python 2.5 is decreasing in popularity, and pip being up to 1.0.x by now, this hopefully won't affect too many folks.

@bitprophet
Member

Silly name brainstorm (all of these are currently not listed on PyPI):

  • Names invoking "something under/part of fabric"
    • Weft
    • Yarn
    • Flax
    • Cotton (eh)
    • Jute
    • Polyester (not really)
  • Names invoking SSH or "secure shell"
    • sshpy (pyssh is taken) (not too serious, I hate 'py' names for the most part)
    • I really can't think of any others that aren't already taken or way too punny.
@zirpu
zirpu commented Sep 28, 2011

i like 'weft'. 'woof' is also good.

@bitprophet
Member

Mentioned on IRC but I'm not a huge fan of 'woof' since it's easily confused with the onomatopeia for the noise a dog makes :D

@bitprophet
Member

I've emailed Robey about this to try and get his side of the story before I pull the trigger; will check back in tomorrow.

@bitprophet
Member

Robey has expressed interest in having me take over Paramiko as he's not got a lot of time for it lately, but unfortunately an attempt at a hand-over is not progressing well (I assume also due to him being busy.)

To try and have my cake and eat it too, I will put up the required modification to Paramiko 1.7.7.1 (via my Github fork of the official repo) on PyPI under a new name, but without renaming the Python package itself or making any other changes (i.e. just the PyPI label/name will change.)

I feel having it on PyPI is preferable over a raw Github tarball URL, and future-proofs us in the event that Robey and I don't work things out re: handover. But by not jumping right into a "real" fork re: package name, copyright additions etc, I'm hoping that the impact on users will then be minimal if the official project does change hands.

Will use 'weft' for now, as it's the least silly name that we came up with -- even though it sounds like Elmer Fudd giving driving directions XD


EDIT: Scratch that -- if I'm leaving the package name the same, it could cause import problems with folks who already have Paramiko installed, which would be just about everybody who's upgrading. Would be the same as when we vendorized Paramiko, re: installing Fabric resulting in clobbering any installed Paramiko on PYTHONPATH. If it even worked.

It sounds like the most problem-free approaches would be a hard fork which renames the package, or forgoing forking entirely in favor of vendorizing. Either approach requires updating my copy of Paramiko (to use the name, or to ensure its internal imports are relative, respectively.)


EDIT 2: also realized that a rename could mean patch sharing between upstream and a fork is much more prone to merge conflicts; especially as Paramiko uses a moderate amount of import paramiko; ...; paramiko.xyz. Maybe I just don't give git enough credit but I assume it would be highly problematic.

A vendorized approach that just updates import lines might be lower-impact; or a fork that does import weft as paramiko, which is silly but might work.

OTOH, given the point here is that Paramiko rarely updates anyways, this is probably premature optimization.

@bitprophet
Member

N.B. any rename will require updating some precomputed hash values, whose inputs include string literals containing "paramiko". The tests which fail are all in tests/test_kex.py and the strings involved are:

  • <package>/transport.py +197: _CLIENT_ID
  • <package>/message.py +68: return <package>.Message ...
  • tests/test_kex.py +52: local_version

Probably best to still include those strings in the search and replace, and to update any "broken" hash values with the new outputs.

@bitprophet
Member

More name BS:

  • ssh: super simple, does what it says on the tin, I own it on PyPI now thanks to @kennethreitz championing the idea.
    • But I worry it might be somewhat confusing in various contexts (discussions, Fabric issues, Googling, etc) re: folks not knowing if the phrase "ssh" refers to the command line program, the protocol, or now this library/package.
      • The counter-argument is that we "just" need to be careful to refer to the library always as "the 'ssh' (python) library" and never just "ssh". Which might work.
  • weft: from above, references "fabric", relatively unique, but suffers from the same problem as "fabric" itself does re: Googling.
  • grasshopper: not taken on pyPI, contains "ssh", otherwise inoffensive. (Seems to conflict with some Rhino modeling tool, though, which comes up in googles for "python grasshopper" as it supports Python as a scripting lang.)

Given none of these are perfect I am still leaning towards ssh.

@bitprophet
Member

Say hi to the Python ssh library. Name change, version change, copyright & author addition/change, and of course the required-for-#19 fix applied and tested.

Will be pushing changes to Fabric to rely on this library, soon. Then the doc changes listed at top of #19, a little bit more testing on various Pythons + platforms (only tested on OS X 10.7, Python 2.5 and 2.7 so far [EDIT: and 2.6 on Ubuntu 10.04]) and then I will put out 1.3.0 which will rely on this fork, and we'll see how many install problems crop up.

@bitprophet bitprophet added a commit that referenced this issue Oct 23, 2011
@bitprophet bitprophet s/paramiko/ssh/g
Re #275, re #19
b7788f5
@kennethreitz

🍰

@bitprophet
Member

Forgot to re: this ticket in 253b905...


At this point I need to take all related tickets and comments and spin them off into an issue tracker for the new ssh fork. I will leave it open until this is taken care of, even though it's listed in the 1.3 changelog (implying that it's "done".)

@bitprophet
Member

This can be closed now, all related issues on our side:

  • are still open, but firmly related to both Fabric and ssh, so they can stay here
  • have been closed as fixed/invalid
  • were just now moved over to the ssh tracker.
@bitprophet bitprophet closed this Feb 29, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment