Skip to content
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

pythonPackages.aws-lambda-builders: init at 0.2.1 #59089

Merged

Conversation

dhl
Copy link
Contributor

@dhl dhl commented Apr 7, 2019

A tool to compile, build and package AWS Lambda functions.

This is being packaged as part of the effort to update aws-sam-cli.

Motivation for this change
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.

@worldofpeace
Copy link
Contributor

This is being packaged as part of the effort to update aws-sam-cli.

It's usually preferred to open one pr for that kind of purpose. Even if you don't update aws-sam-cli in the pr.

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

See the comments above.

@dhl
Copy link
Contributor Author

dhl commented Apr 8, 2019

If the package looks good to you, I'll remove the change to maintainers.nix and squash the commits.

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Looking at their Makefile I think we should be running the tests like

pytest tests/functional

in the checkPhase.

@worldofpeace
Copy link
Contributor

worldofpeace commented Apr 8, 2019

It also looks like this shouldn't be built on !=3.5.* so we should use the disabled parameter to disable it for that interpreter version

disabled = isPy35

else build fails for python35Packages.aws-lambda-builders

aws-lambda-builders requires Python '>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*, !=3.5.*' but the running Python is 3.5.7

Edit: is coming from https://github.com/awslabs/aws-lambda-builders/blob/b811288dae4598abede2c1daa54cdfd3c5865301/setup.py#L49

@dhl dhl force-pushed the dhl/python-modules/aws-lambda-builders branch from 7a579e2 to 6663765 Compare April 8, 2019 06:31
@dhl
Copy link
Contributor Author

dhl commented Apr 8, 2019

@worldofpeace I've added implemented all the suggested changes and squashed them:

  1. Fix homepage URL
  2. Remove trailing period in description
  3. Fetch from GitHub to get tests
  4. Run functional tests (unit and integration tests not working)
  5. Disabled for python 3.5
  6. Remove adding myself to maintainers.nix (merged in pythonPackages.chevron: init at 0.13.1 #59088)

Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

Builds for every interpreter version ✨

@worldofpeace worldofpeace merged commit fa75874 into NixOS:master Apr 8, 2019
@dhl dhl deleted the dhl/python-modules/aws-lambda-builders branch April 8, 2019 15:07
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.

None yet

3 participants