Skip to content

aws-sam-cli: 0.14.2 -> 0.16.1#62011

Merged
Ma27 merged 3 commits intoNixOS:masterfrom
dhl:dhl/aws-sam-and-lambda-builders
May 25, 2019
Merged

aws-sam-cli: 0.14.2 -> 0.16.1#62011
Ma27 merged 3 commits intoNixOS:masterfrom
dhl:dhl/aws-sam-and-lambda-builders

Conversation

@dhl
Copy link
Member

@dhl dhl commented May 24, 2019

Motivation for this change

To bring aws-sam-cli up-to-date and fix pythonPackages.serverlessrepo, which was made broken by pyyaml being updated to a newer version ni nixpkgs.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@dhl dhl requested a review from FRidh as a code owner May 24, 2019 15:40
@ofborg ofborg bot added 6.topic: python Python is a high-level, general-purpose programming language. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels May 24, 2019
@Ma27
Copy link
Member

Ma27 commented May 24, 2019

Building aws-sam-cli from your branch fails with the following error on my machine:

Collecting aws-sam-translator==1.10.0 (from aws-sam-cli==0.16.1)
  Could not find a version that satisfies the requirement aws-sam-translator==1.10.0 (from aws-sam-cli==0.16.1) (from versions: )
No matching distribution found for aws-sam-translator==1.10.0 (from aws-sam-cli==0.16.1)
builder for '/nix/store/hi4v4y7a2sl24fsf824c9xicazdq9b2h-aws-sam-cli-0.16.1.drv' failed with exit code 1
error: build of '/nix/store/hi4v4y7a2sl24fsf824c9xicazdq9b2h-aws-sam-cli-0.16.1.drv' on 'ssh://builder' failed: builder for '/nix/store/hi4v4y7a2sl24fsf824c9xicazdq9b2h-aws-sam-cli-0.16.1.drv' failed with exit code 1

@dhl dhl force-pushed the dhl/aws-sam-and-lambda-builders branch from 81023d5 to 47e31ab Compare May 25, 2019 01:40
@dhl
Copy link
Member Author

dhl commented May 25, 2019

Thanks for reviewing this, @Ma27!

I forgot to rebuild the aws-sam-cli after rebase, and aws-sam-translator was also updated.

I've updated aws-sam-cli to pin aws-sam-translator at 0.10.0

@Mic92
Copy link
Member

Mic92 commented May 25, 2019

@GrahamcOfBorg build aws-sam-cli

@Ma27
Copy link
Member

Ma27 commented May 25, 2019

Built and tested locally, nix-review seems fine 👍

@Ma27 Ma27 merged commit 35961a1 into NixOS:master May 25, 2019
@Ma27
Copy link
Member

Ma27 commented May 25, 2019

@dhl thanks!

@dhl
Copy link
Member Author

dhl commented May 25, 2019

Thank you @Ma27!

@dhl dhl deleted the dhl/aws-sam-and-lambda-builders branch May 25, 2019 11:16
FRidh
FRidh reviewed May 25, 2019
View reviewed changes
Copy link
Member

@FRidh FRidh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be reverted.

pkgs/development/python-modules/serverlessrepo/default.nix
let
py = python.override {
packageOverrides = self: super: {
pyyaml = super.pyyaml.overridePythonAttrs (oldAttrs: rec {
Copy link
Member

@FRidh FRidh May 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overriding a Python library inside python-packages.nix is going to cause trouble when composing an env consisting of the original version and the overridden version

Copy link
Member Author

@dhl dhl May 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FRidh understood. I'll get a fix in ASAP.

serverlessrepo has a dependency on pyyaml declared as 'pyyaml~=3.12'.

What's the best way to deal with this? Should I fix up the version in setup.py, or package an older version of pyyaml?

Copy link
Member

@FRidh FRidh May 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a) check (yourself/with upstream) whether it actually does work with the version we use, and if so patch it
b) if it does not work, then the module is unfortunately broken. Even so, an application can choose to perform the override as is done here

Copy link
Member Author

@dhl dhl May 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @FRidh. The tests are passing for python 2.7 after patching. Will make another PR after the tests also pass for 3.5, 3.6 and 3.7.

Apologies for the trouble.

Copy link
Member

@Ma27 Ma27 May 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, I actually knew about this, but totally missed that the change is done to a library, sorry! :/

@dhl do you need assistance to fix that?

Copy link
Member Author

@dhl dhl May 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ma27 I've fixed this in #62051

@FRidh
Copy link
Member

FRidh commented May 25, 2019

We can only perform overrides of run-time dependencies in applications, not in libraries.

@dhl dhl mentioned this pull request May 25, 2019
Merged
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Ma27 Ma27 Ma27 left review comments

+1 more reviewer

@FRidh FRidh FRidh left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dhl @Ma27 @Mic92 @FRidh