aws-sam-cli: avoid hard-coded requests version (use the nix package version) #67286
Conversation
There was a problem hiding this comment.
nix-review doesn't rebuild anything
I'm a little weary since we don't have tests, but it's unlikely that those tests would do much anyway since requests package does network I/O.
However, it might be good to have an assertion that the "requests" package is less than 3. Something like assert lib.versionOlder ${requests.version} "3". But at the point we may as well just do
substituteInPlace requirements/base.txt --replace "requests==2.20.1" "requests<3"
|
I'm certainly OK with changing it to |
|
Seems that the requests library is faithful to SemVer according to https://2.python-requests.org/en/master/dev/philosophy/#semantic-versioning . So i think it's safe to do the However, I also want to point out that the left side of the replace needs to match exactly each time they decide to bump it as well. It might be worth while to do just a sed -i replace line command like: |
|
I opened a PR to fix this upstream, I don't blame them for being adverse to having more flexible versioning, but it would make life easier. |
|
I've been feeling guilty for neglecting this package, and seeing this gives me a rush of relief. Thank you @scoates and @jonringer! |
| @@ -56,7 +56,7 @@ buildPythonApplication rec { | |||
| ]; | |||
|
|
|||
| postPatch = '' | |||
| substituteInPlace requirements/base.txt --replace "requests==2.20.1" "requests==2.22.0" | |||
| substituteInPlace requirements/base.txt --replace "requests==2.20.1" "requests==${requests.version}" | |||
There was a problem hiding this comment.
Our version should always be the latest, i say we should just use the compatible operator, and be done with it. (It's the same as saying requests>=2.2,2.*. So it will break if requests ever releases a major bump, but should work otherwise. This should prevent us from needing to touch this ever again)
| substituteInPlace requirements/base.txt --replace "requests==2.20.1" "requests==${requests.version}" | |
| sed -i '/requests==2/c\requests~=2.2' requirements/base.txt |
|
Thank you for your contributions.
|
stale
bot
added
the
2.status: stale
|
I should take myself off the maintainers. I am not using this package anymore |
stale
bot
removed
the
2.status: stale
|
I marked this as stale due to inactivity. → More info |
stale
bot
added
the
2.status: stale
|
Since aws-sam-cli has advanced to a version much higher than it was at the point this PR was made, I assume it can be closed. Please resubmit or rebase if there's still anything of value. |
Motivation for this change
aws-sam-cli had a hard-coded number dependency on pythonModule.requests's version. This small PR makes it use
${requests.version}instead (thanks to a pointer from @grahamc).This package has been out of date several times for me (today is the first time I knew how to fix it); this should make it forward-compatible.
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nix-review --run "nix-review wip"./result/bin/)nix path-info -Sbefore and after)Notify maintainers
cc @andreabedini @dhl