Skip to content

Loosen requests version bounds#1363

Closed
jonringer wants to merge 1 commit intoaws:developfrom
jonringer:loosen-requests-bounds
Closed

Loosen requests version bounds#1363
jonringer wants to merge 1 commit intoaws:developfrom
jonringer:loosen-requests-bounds

Conversation

@jonringer
Copy link

Issue #, if available:
Nope

Description of changes:
Use compatible versions of requests instead of pinning. Requests respects SemVer according to https://2.python-requests.org/en/master/dev/philosophy/#semantic-versioning , so there's no need to pin to a specific version. Anything in the >=2.22,<3 range should be compatible.

Checklist:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sanathkr
Copy link
Contributor

We pin to specific versions for 1) determinism of builds and 2) test & validate SAM CLi works with this version.

Can you explain why you want to expand this?

@jonringer
Copy link
Author

jonringer commented Aug 22, 2019

  1. determinism of builds and 2) test & validate SAM CLi works with this version.

For tracing errors back to a given dependency, this makes sense. However, for many people, they most likely aren't just installing aws-sam-cli alone, but rather as a series of other packages. If someone has an environment where another package does similar pinning to requests==2.21, then they will get an error.

How about instead i talk about the benefits of using compatible versions with pip:

  • Pip will choose the latest available version (in the version bound constraint) so you should be getting the most-up-to-date package
  • Dont have to worry about CVE's which were patched in later versions and get PR's trying to fix them like fix: upgrade requests to 2.22.0 #1200
  • Version breathing room so that some flexibility allows you to play nicely with other packages. (Another package may have requests~=2.23 when it gets released)

Not all packages follow SemVer, however, requests does, so we can leverage some of those guarantees of compatibility.

As an aside, requests is the only non-aws dependency that is pinned:

six~=1.11.0
chevron~=0.12
click~=7.0
enum34~=1.1.6; python_version<"3.4"
Flask~=1.0.2
boto3~=1.9, >=1.9.56
PyYAML~=5.1
cookiecutter~=1.6.0
aws-sam-translator==1.13.1
docker~=4.0
dateparser~=0.7
python-dateutil~=2.6
pathlib2~=2.3.2; python_version<"3.4"
requests==2.22.0
serverlessrepo==0.1.9
aws_lambda_builders==0.3.0

My original use case was packaging aws-sam-cli with nixpkgs, we supply the python package version that is available through nixpkgs (usually the latest). And since the dependency requests is pinned, we have to adjust for the bumps.

Examples:

@jfuss
Copy link
Contributor

jfuss commented Aug 27, 2019

@jonringer Customers will only run into what you described (dependency conflicts) if they are installing multiple different projects (libraries, clis, etc) into the same python environment. When doing this, you will run the risk (if not us someone else). You are using the same concepts of a library with a CLI.

As a library author, you will want to have loose(r) dependency graphs so that the end user of your library can declare what version they need. When we talk about a CLI, that is no longer needed. Loose dependencies allow for dependencies to change at runtime, which makes the CLI itself more fragile. One of the best practices in the python is to use a venv to isolate installation of tools. This gives us (or other tools) a way to be completely isolated and therefore ensure what we release is in working order.

We are only really talking about this because the cli is written in Python and Python itself has some quirks. If the cli was written in node or go for example, you would be getting this by default.

Given we are not a library, I don't think relating dependencies is the right thing to do here. Instead, can you guys install using a venv instead or better yet use brew?

@jonringer
Copy link
Author

@jonringer Customers will only run into what you described (dependency conflicts) if they are installing multiple different projects (libraries, clis, etc) into the same python environment. When doing this, you will run the risk (if not us someone else).

I feel like having an environment with aw-sam-cli, their problem-specific packages, plus related python packages a completely normal scenario, and probably the norm in most user cases. Having a single environment avoids the need of switching contexts constantly.

You are using the same concepts of a library with a CLI.

Dependencies are likely to break you in both scenarios. If this is true, then why are all of the other non-aws dependencies in a range of compatible versions?

As a library author, you will want to have loose(r) dependency graphs so that the end user of your library can declare what version they need. When we talk about a CLI, that is no longer needed. Loose dependencies allow for dependencies to change at runtime, which makes the CLI itself more fragile.

As an end user, you want the most recent compatible version, so that security vulnerabilities and patches can be applied without breaking you. One of the many reasons why the ~= operator exists.

One of the best practices in the python is to use a venv to isolate installation of tools. This gives us (or other tools) a way to be completely isolated and therefore ensure what we release is in working order.

venv is a bandaid, and doesn't help with dependency composition with other packages. Having some flexibility with version bounds does remedy this somewhat. As long as python has PYTHONPATH + site-packages as their dependency abstraction, this will always be an issue.

We are only really talking about this because the cli is written in Python and Python itself has some quirks. If the cli was written in node or go for example, you would be getting this by default.

Sure, but doesn't really mean you should avoid SemVer. Plus, those ecosystems have their own tradeoffs (node usually has dependency bloat, and go has no versioning at all).

Instead, can you guys install using a venv instead or better yet use brew?

Nix is essentially a system-wide venv+package manager where all packages and their dependencies are enclosed from each other in different scopes/closures/environments. It differs from the other package managers because it takes reproducibility and determinism to the extreme. So, it competes with brew, AUR, and others.

The nix build pipeline has the ability to alter any of the source code before installation, which is what my examples in the previous post were demonstrating. The point of this PR was to end the need to search+replace the request version (while requests~=2.2), since there's a delay when it gets updated in our packages and your bump+release.

@jfuss
Copy link
Contributor

jfuss commented Aug 27, 2019

I feel like having an environment with aw-sam-cli, their problem-specific packages, plus related python packages a completely normal scenario, and probably the norm in most user cases. Having a single environment avoids the need of switching contexts constantly.

You do not need to switch context, you would add to the PATH to avoid this.

Dependencies are likely to break you in both scenarios. If this is true, then why are all of the other non-aws dependencies in a range of compatible versions?

We are moving toward pinning everything: #1369

As an end user, you want the most recent compatible version, so that security vulnerabilities and patches can be applied without breaking you. One of the many reasons why the ~= operator exists.

Again, this is mainly for a library. As a CLI and pinning everything, this becomes a responsibility of us.

venv is a bandaid, and doesn't help with dependency composition with other packages. Having some flexibility with version bounds does remedy this somewhat. As long as python has PYTHONPATH + site-packages as their dependency abstraction, this will always be an issue.

Not sure I understand this. We vend the CLI not a library, so the dependency composition is for our dependencies. To give isolation and reproducibility, we are moving (and pin some currently) towards pinning all dependencies. Venv gives you the isolation so that the cli can be installed and not changed from under you (which has broke us in the past).

The nix build pipeline has the ability to alter any of the source code before installation, which is what my examples in the previous post were demonstrating. The point of this PR was to end the need to search+replace the request version (while requests~=2.2), since there's a delay when it gets updated in our packages and your bump+release.

We get into the same reproducibility issues. Yes, this is just a minor version patch (I get it) but we use the source to build our MSIs, brew bottles, etc, which we need to control what gets bundled. Allowing patch versions means things can change, for example a new dependency is added to our direct dependency with some license restrictions.

I understand the intention here but you are using the fact that it is written in python. As a consumer of the cli, you should never care about what the cli is written in. This is we have and will continue to invest in installers to allow better installations for all. Maybe one of the reasons we are in this position is because we don't have a universal installer (not sure if Nix is cross platform) that you can use instead?

@jonringer
Copy link
Author

You do not need to switch context, you would add to the PATH to avoid this.

Simply modifying my path and not the PYTHONPATH would result in ImportError's when your cli tries to do something like import yaml

We are moving toward pinning everything: #1369

I think this is a step back. if you simply wanted to export a reproducible cli, that plays well with the rest of the ecosystem, then python wasn't the right choice. (I'm aware that the other aws libraries made it a non-choice)

Again, this is mainly for a library. As a CLI and pinning everything, this becomes a responsibility of us.

I don't have context on any of the issues you've faced with breakages due to dependency versions, but I feel like it's a self-induced prison.

Not sure I understand this. We vend the CLI not a library, so the dependency composition is for our dependencies.

I'm talking about pip install <some other problem-specific packages>; some work... ; pip install aws-sam-cli

Hmm, sounds like you all are moving more toward the view of pinning everything, instead of using compatible versions. I'll close the PR.

@jonringer jonringer closed this Aug 27, 2019
@jfuss jfuss reopened this Aug 29, 2019
@jfuss
Copy link
Contributor

jfuss commented Aug 29, 2019

@jonringer We did some deeper thinking and discussions about the pinning and how we want to handle this. What we ended up on is creating a new requirements.txt file that we could solely use for our 'binary' distributions. What this gives is people using pip (even though strongly advise not to), can still install through that for now and we still get a way to create a full reproducible build.

We are looking to invest more into our installations so hopefully Nix will be able to use some non-pip way in the future.

@jonringer
Copy link
Author

now have merge conflicts, and I've lost interest in up-streaming semver versioning

@jonringer jonringer closed this Nov 18, 2019
@jonringer jonringer deleted the loosen-requests-bounds branch November 18, 2019 21:45
@mbbx6spp
Copy link

@jfuss I wasted hours from bad error messages from sam because I was on an old version, only because the newer version that support authorizers can't run for me. Can you switch to building fully self-contained executables to avoid this? It is a CLI tool after all and users shouldn't have to care exactly how the CLI tools they use are packaged.

@jonringer
Copy link
Author

Python doesn't have an officially supported way of creating an executable, and the "bundlers" that I'm aware of will create massive binaries.

Unfortunately python's module system is just "blindly import stuff at runtime", which means you can't really separate out what's exposed. The bandaid is to use "environments" (conda/venv) to at least mitigate the packages to a specific project, but if you want a global install, then you need to "play well" with the rest of the python exosystem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants