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

Consider bumping Paramiko requirement pin once Paramiko 2.0.x is definitely stable #1461

Closed
bitprophet opened this Issue Apr 30, 2016 · 19 comments

Comments

Projects
None yet
5 participants
@bitprophet
Member

bitprophet commented Apr 30, 2016

  • We put out 1.10.3 and 1.11.1 with explicit paramiko<2 pinning to prevent peoples' environments from unexpectedly upgrading / breaking, a while back.
  • Paramiko 2.0 is now out but is fresh, presumably might be some bugs that need ironing out.
  • It would be nice to allow people to upgrade to Paramiko 2.0 under Fabric 1 if they explicitly want it - however I don't see a way in setuptools/pip to do this
    • e.g. the extras_require functionality doesn't let you override the same package/version as specified in install_requires, so we can't do pip install fabric[paramiko-2] or anything.
    • Hopefully there's something else we can do that I am missing, like setup.py CLI parameters - but those are also not a panacea as they do not work with wheels or any other non-sdist installation method
    • I'd also like to avoid having to make some terrible second PyPI entry / setup.py like fabric-paramiko2 though that is another option.
  • Given that Paramiko 2 is API compatible with Paramiko 1, we may want to just bump the version pin to paramiko<3 in later Fabric releases, so folks can continue getting bug/security/etc fixes from the Paramiko 2.x line
    • If we do this, need to remove the Crypto.atfork call in decorators.py (see #1460) and make sure no other PyCrypto baggage remains
    • This will still always have a chance of breaking for people who can't get the all-in-one Cryptography.io static wheel; they will upgrade Fabric and then it will explode if they lack libffi-dev or openssl-dev. So it'd be entirely sensible to never actually undo this pin, instead having folks use Fabric 2 once it's out.
@alex

This comment has been minimized.

alex commented May 1, 2016

Perhaps consider an intermediary state, where for one or two releases, the fabric setup.py allows both paramiko>=2 and paramiko<2, and people will get cryptography by default, but can force the old PyCrypto version if they need it. Once that's burned in a little, you can move setup.py to require paramiko>=2.

@bitprophet

This comment has been minimized.

Member

bitprophet commented May 5, 2016

Not sure I'd ever make Fabric 1.x require paramiko>=2, planning to save that for Fabric 2.x. Depends on Fabric 2.x uptake once it's out I guess.

You're right that going back to unpinned "I don't care, paramiko 1 or 2 is ok" is still solvable by letting folks manually downgrade their Paramiko. the pinning was mostly an attempt to head off a flood of "onoz u broke my build" reports. We've gotten some, but not a ton, whether that was due to my pin or not is anybody's guess tho.

@alex

This comment has been minimized.

alex commented May 7, 2016

@bitprophet FWIW, I have some dataz! Over the past 2 weeks (so this includes a few days before paramiko 2.0 was actually released), here are the most downloaded Paramiko versions from PyPI:

Version Downloads
1.16.0 411903
2.0.0 308131
1.17.0 77360
1.15.2 47677
1.15.1 23893

To me this very large number of 2.0 installs indicates that it's probably safe, and so removing the <2 bound is unnecessary, for most people it'll work just fine (or be easily remediated), and for those who can't simply doing a pip install paramiko<2 before pip install fabric will fix it.

@bitprophet bitprophet added this to the 1.12 milestone Jun 4, 2016

@Julian

This comment has been minimized.

Julian commented Jun 29, 2016

Would it be possible at least to use an environment marker to tell fabric to use paramiko>2 for PyPy at least, where the current status quo is that fabric won't install anyhow?

@bitprophet

This comment has been minimized.

Member

bitprophet commented Jul 25, 2016

@alex belatedly, did you mean "removing the <2 bound is safe"? (instead of "unnecessary") :D

@Julian I assume you mean another few lines of "alter install_requires based on current interpreter"? Not opposed to that offhand. (Though IIRC such hacks stop working with wheels so we're starting to shy away from them...?)

@alex

This comment has been minimized.

alex commented Jul 25, 2016

Uhhh, yes, I meant it's safe.

@Julian

This comment has been minimized.

Julian commented Jul 25, 2016

@bitprophet for wheels you do the same thing, just in extras_require with an environment marker.

Basically you rub @dstufft's belly and a genie pops out.

(More serious example: https://github.com/Julian/jsonschema/blob/master/setup.py#L25 but you replace python_version with python_interpreter to dispatch on that instead).

I'll try to put that in a PR so you can see how it looks, unless you're about to make this happen either way?

@bitprophet

This comment has been minimized.

Member

bitprophet commented Jul 25, 2016

Oh neat, I only vaguely remember learning about that stuff a while back. Clearly then forgot. I'm definitely down with doing that if it'll help some folks & not harm the majority. A PR would be appreciated.

Re: outer issue, I think at this point I'm down with bumping the pin to <3 in Fabric 1.12+ (alongside the other Crypto purges mentioned earlier, though they'd have to become conditionals unless I wanted to do paramiko>=2,<3, which I am less in favor of. Will see how ugly things get when I poke this and try making sure the same Fabric branch passes tests in two separate venvs, one w/ Crypto and Paramiko 1, the other with no Crypto and Paramiko 2)

@hostep

This comment has been minimized.

hostep commented Jul 26, 2016

Hi guys

Small addition to this discussion:
We use Macports to install the Fabric port which depends on the Paramiko port on our macOS workstations.
But recently Macports decided to upgrade Paramiko from version 1.16.0 to 2.0.1 and when we now run Fabric, it no longer works:

➜ fab deploy
Traceback (most recent call last):
  File "/opt/local/bin/fab", line 5, in <module>
    from pkg_resources import load_entry_point
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2927, in <module>
    @_call_aside
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2913, in _call_aside
    f(*args, **kwargs)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/pkg_resources/__init__.py", line 2940, in _initialize_master_working_set
    working_set = WorkingSet._build_master()
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/pkg_resources/__init__.py", line 637, in _build_master
    return cls._build_from_requirements(__requires__)
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/pkg_resources/__init__.py", line 650, in _build_from_requirements
    dists = ws.resolve(reqs, Environment())
  File "/opt/local/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/site-packages/pkg_resources/__init__.py", line 829, in resolve
    raise DistributionNotFound(req, requirers)
pkg_resources.DistributionNotFound: The 'paramiko<2.0,>=1.10' distribution was not found and is required by Fabric

I got around it by downgrading Paramiko to version 1.16.0 using Macports, which was a bit complicated to do manually, but eventually got it working again.

I think this is a mistake from Macports, they should have waited to upgrade Paramiko in their tree until Fabric was compatible.

But anyway, it would be great if a new version of Fabric can be released with support for Paramiko > 2.0 so we can get everything to work again using Macports with the latest versions of all the ports.

@bitprophet

This comment has been minimized.

Member

bitprophet commented Jul 26, 2016

Grump, that's annoying - thanks for the news, @hostep. (Not trying to be snarky, but - also surprised MacPorts is still in use, everybody I know switched to Homebrew years ago. Good reminder that "top of mind" != "the only game in town" I guess :))

I popped out a tiny 1.12 on a whim last night (another good reminder to self: stop using literal version numbers when discussing roadmap - just say "an upcoming feature release" or etc) and didn't make this change, but still want to do it soon, so probably the next minor release instead.

@koobs

This comment has been minimized.

koobs commented Nov 10, 2016

FWIW, we (FreeBSD) experienced the same issue and had to create a paramiko1 port and package for py-fabric to depend on. See Also:

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213893
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=214379

TLDR: <= and == dependencies are super painful/annoying and create more problems than they purport to solve.

There is much less pain for downstream users and packagers/porters when upstreams just proactively test against the latest versions of their dependencies (in development, in advance of releases is the best time to do this) and don't prescribe or constraint dependency versioning unless there are versions which explicitly break compatibility.

Even in that case the limitation is only temporary, and only if a release is made while its broken, and only while the dependencies author(s) is/are notified and until fixes are made.

The bonus of this method is it teaches upstreams (and upstreams of upstreams) that the decisions they make actually affect their users in practice (not just in theory), and with this knowledge, hopefully minimises the likelihood of it happening again in future.

@hostep

This comment has been minimized.

hostep commented Nov 10, 2016

To add a bit more info regarding the Macports issue, it seems to be fixed because they added a patch while installing which changes the requirement for Paramiko from <2.0 to <3.0
We now run Fabric v1.12.0 & Paramiko v2.0.2 on our macOS workstations and have no problems with this.

@koobs

This comment has been minimized.

koobs commented Nov 10, 2016

@hostep Thank you for highlighting that

@koobs

This comment has been minimized.

koobs commented Nov 10, 2016

@hostep Indeed there are times where I've overridden *_requires specs (from <=/== to >= or '') in FreeBSD ports, if the test suite has passed with a later version of the dependency. This however relies on the awesome of the tests so can be risky, and though we're a downstream of Fabric, we don't like to break our user (downstream) environments either :)

bitprophet referenced this issue Dec 1, 2016

@bitprophet

This comment has been minimized.

Member

bitprophet commented Dec 6, 2016

More numbers updates...last 6 month overview:

screen shot 2016-12-05 at 6 27 27 pm

and last 2 weeks:

screen shot 2016-12-05 at 6 35 09 pm

Paramiko 2.0.x now easily half of all downloads. And I'm wondering how much of the other 50% are due to this ticket not being merged :) will find out soon!

@bitprophet

This comment has been minimized.

Member

bitprophet commented Dec 9, 2016

Think I'll execute on this and pop it out as Fab 1.13.0.

Revisited what @Julian and I discussed way back but that extras_require magic seems only necessary if I wanted to limit the change to PyPy; at this point I'm just doing the "allow Paramiko <3" bit wholesale...

@bitprophet bitprophet closed this in 319db0a Dec 9, 2016

@bitprophet

This comment has been minimized.

Member

bitprophet commented Dec 9, 2016

On PyPI

@bitprophet

This comment has been minimized.

Member

bitprophet commented Dec 9, 2016

Gah, of course I forgot the sibling ticket, #1462! 1.13.1 on the way...EDIT: and, merged/released.

@Julian

This comment has been minimized.

Julian commented Dec 9, 2016

uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Jul 15, 2017

koobs
devel/py-fabric: Update to 1.13.2
- Switch paramiko1 to paramiko (2.x) in RUN_DEPENDS, it is now supported [1]

Changelog:

  http://www.fabfile.org/changelog.html

[1] fabric/fabric#1461
> Description of fields to fill in above:                     76 columns --|
> PR:                       If and which Problem Report is related.
> Submitted by:             If someone else sent in the change.
> Reported by:              If someone else reported the issue.
> Reviewed by:              If someone else reviewed your modification.
> Approved by:              If you needed approval for this commit.
> Obtained from:            If the change is from a third party.
> MFC after:                N [day[s]|week[s]|month[s]].  Request a reminder email.
> MFH:                      Ports tree branch name.  Request approval for merge.
> Relnotes:                 Set to 'yes' for mention in release notes.
> Security:                 Vulnerability reference (one per line) or description.
> Sponsored by:             If the change was sponsored by an organization.
> Differential Revision:    https://reviews.freebsd.org/D### (*full* phabric URL needed).
> Empty fields above will be automatically removed.

M    py-fabric/Makefile
M    py-fabric/distinfo


git-svn-id: svn+ssh://svn.freebsd.org/ports/head@445866 35697150-7ecd-e111-bb59-0022644237b5

uqs pushed a commit to freebsd/freebsd-ports that referenced this issue Jul 15, 2017

devel/py-fabric: Update to 1.13.2
- Switch paramiko1 to paramiko (2.x) in RUN_DEPENDS, it is now supported [1]

Changelog:

  http://www.fabfile.org/changelog.html

[1] fabric/fabric#1461
> Description of fields to fill in above:                     76 columns --|
> PR:                       If and which Problem Report is related.
> Submitted by:             If someone else sent in the change.
> Reported by:              If someone else reported the issue.
> Reviewed by:              If someone else reviewed your modification.
> Approved by:              If you needed approval for this commit.
> Obtained from:            If the change is from a third party.
> MFC after:                N [day[s]|week[s]|month[s]].  Request a reminder email.
> MFH:                      Ports tree branch name.  Request approval for merge.
> Relnotes:                 Set to 'yes' for mention in release notes.
> Security:                 Vulnerability reference (one per line) or description.
> Sponsored by:             If the change was sponsored by an organization.
> Differential Revision:    https://reviews.freebsd.org/D### (*full* phabric URL needed).
> Empty fields above will be automatically removed.

M    py-fabric/Makefile
M    py-fabric/distinfo

mat813 pushed a commit to mat813/freebsd-ports that referenced this issue Jul 17, 2017

koobs
devel/py-fabric: Update to 1.13.2
- Switch paramiko1 to paramiko (2.x) in RUN_DEPENDS, it is now supported [1]

Changelog:

  http://www.fabfile.org/changelog.html

[1] fabric/fabric#1461
> Description of fields to fill in above:                     76 columns --|
> PR:                       If and which Problem Report is related.
> Submitted by:             If someone else sent in the change.
> Reported by:              If someone else reported the issue.
> Reviewed by:              If someone else reviewed your modification.
> Approved by:              If you needed approval for this commit.
> Obtained from:            If the change is from a third party.
> MFC after:                N [day[s]|week[s]|month[s]].  Request a reminder email.
> MFH:                      Ports tree branch name.  Request approval for merge.
> Relnotes:                 Set to 'yes' for mention in release notes.
> Security:                 Vulnerability reference (one per line) or description.
> Sponsored by:             If the change was sponsored by an organization.
> Differential Revision:    https://reviews.freebsd.org/D### (*full* phabric URL needed).
> Empty fields above will be automatically removed.

M    py-fabric/Makefile
M    py-fabric/distinfo


git-svn-id: https://svn.freebsd.org/ports/head@445866 35697150-7ecd-e111-bb59-0022644237b5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment