awsebcli: init at 3.10.5 #28428

eqyiel · 2017-08-21T00:36:31Z

Motivation for this change

I wanted to use awsebcli at work and it wasn't in nixpkgs yet.

Things done

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
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 nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

Also:

moved colorama_3_7 from the awscli derivation into python-packages since both awscli and awsebcli depend on it.
refactored some of the parent derivations that I needed to override in order to use fetchPypi, which AFAICT is preferred for python packages (3to2: init at 1.1.1 #25202 (comment), 3to2: refactor to use fetchpypi #26078).
I tried to find what is the current best practice regarding versioning derivation names but it appears that although there is a section in the manual about it there is a lot of bikeshedding still (standardize multi-versioning suffix pattern in all-packages.nix #17625), so I just went with what I think is sensible based on the names of other derivations in pythonPackages (package_V_V).

eqyiel · 2017-08-28T23:25:30Z

Rebased!

FRidh

package_V_V is indeed the way forward.

FRidh · 2017-08-29T07:55:41Z

pkgs/development/python-modules/blessed/default.nix

@@ -0,0 +1,21 @@
+{ stdenv, fetchurl, pythonPackages }:


No pythonPackages, but parameters for each package/function.

FRidh · 2017-08-29T07:55:50Z

pkgs/development/python-modules/blessed/default.nix

+  pname = "blessed";
+  version = "1.14.2";
+
+  src = fetchurl {


FRidh · 2017-08-29T07:56:32Z

pkgs/tools/virtualization/awsebcli/default.nix

@@ -0,0 +1,94 @@
+{ stdenv, fetchurl, pythonPackages }:
+let
+  botocore = pythonPackages.botocore.overrideAttrs (oldAttrs: rec {


overridePythonAttrs (this is quite new)

Furthermore, due to the amount of overrides and the possible need of doing deep overrides, i suggest overriding the package set. E.g.

let mypython = python.override { packageOverrides = self: super: { botocore = super.botocore.overridePythonAttrs (oldAttrs: { ... }); docker = super.docker.overridePythonAttrs( oldAttrs: { ... }); }; }; in ...

FRidh · 2017-08-29T07:56:56Z

pkgs/tools/virtualization/awsebcli/default.nix

+      };
+    });
+
+  makePypiUrl = { pname, version }:


FRidh · 2017-08-29T07:59:53Z

pkgs/development/python-modules/cement/default.nix

@@ -0,0 +1,22 @@
+{ stdenv, pythonPackages }:


No pythonPackages, but parameters for each package/function.

FRidh · 2017-08-29T08:06:30Z

pkgs/top-level/python-packages.nix

@@ -3093,6 +3086,17 @@ in {

  colorama = callPackage ../development/python-modules/colorama { };

+  # Needed by awscli and awsebcli.
+  colorama_3_7 = self.colorama.overrideAttrs (oldAttrs: rec {


overridePythonAttrs

eqyiel · 2017-09-01T00:31:18Z

@FRidh thank you for the review, I've tried to address those points and updated the PR.

Overriding the package set is a lot nicer!

I also rebased onto master again.

I plan to add myself as a maintainer for the new packages and update to 3.10.6 in separate commits, is that the convention here?

FRidh

I plan to add myself as a maintainer for the new packages and update to 3.10.6 in separate commits, is that the convention here?

Actually, the convention is one commit per expression you touch.

FRidh · 2017-09-01T05:38:06Z

pkgs/tools/virtualization/awsebcli/default.nix

+  localPythonPackageSet = python.override {
+    packageOverrides = self: super: rec {
+      docker = super.docker.overridePythonAttrs (oldAttrs: rec {
+          pname = "docker-py";


you can use oldAttrs

Unfortunately the name of the Pypi package changed in 2.0: https://github.com/docker/docker-py/releases/tag/2.0.0

docker-py is the name of the package prior to that but the pname from oldAttrs is docker.

Unless you're saying to use ${oldAttrs.pname}-py?

FRidh · 2017-09-01T05:38:23Z

pkgs/tools/virtualization/awsebcli/default.nix

+    packageOverrides = self: super: rec {
+      docker = super.docker.overridePythonAttrs (oldAttrs: rec {
+          pname = "docker-py";
+          version = "1.7.2";


note that you will have to update name as well yourself

FRidh · 2017-09-01T05:39:59Z

pkgs/tools/virtualization/awsebcli/default.nix

+    sha256 = "1g53z2flhp3navdf8lw6rgh99akf3k0ng1zkkqswvh66zswkxnwn";
+  };
+
+  buildInputs = with localPythonPackageSet.pkgs; [


these are likely inputs needed only for testing. The buildPython* functions supports checkInputs.

Thanks, i wasn't aware that was a thing

Its only used in buildPython*, and I think only exists since 17.03.

eqyiel · 2017-09-01T12:05:06Z

Actually, the convention is one commit per expression you touch.

I've split it out into more atomic commits, is this preferable? 😊

FRidh

I've split it out into more atomic commits, is this preferable?

Yep, much nicer this way!

FRidh · 2017-09-02T07:57:12Z

pkgs/tools/virtualization/awsebcli/default.nix

+        };
+      });
+
+      docker = super.docker.overridePythonAttrs (oldAttrs: rec {


am I correct that you are now using a different package for docker? That is, docker-py instead of docker?

It's the same package from the same repo (https://github.com/docker/docker-py) but since version 2.0.0 (the version in pkgs/development/python-modules/docker.nix is 2.5.1) they changed the name that you would use to install it with pip.

It's in the release notes here: https://github.com/docker/docker-py/releases/tag/2.0.0

The name of the pip package is now docker (was: docker-py). New versions of this library will only be published as docker from now on.

Should I create a new derivation for it?

FRidh · 2017-09-02T07:57:14Z

pkgs/tools/virtualization/awsebcli/default.nix

+      colorama = super.colorama.overridePythonAttrs (oldAttrs: rec {
+        version = "0.3.7";
+
+        src = fetchPypi {


Note that fetchPypi is overridable, so you can write

src = oldAttrs.src.override { inherit version; sha256 = "..."};

No need to change it, though.

FRidh · 2017-09-02T07:58:26Z

pkgs/tools/virtualization/awsebcli/default.nix

+      docker = super.docker.overridePythonAttrs (oldAttrs: rec {
+          pname = "docker-py";
+          version = "1.7.2";
+          name = "${pname}-${version}";


This line is necessary for all the overrides since otherwise the old names will still be used.

Hopefully, after the release of 17.09, we can go further and get rid of name for buildPython* :)

Thanks for letting me know!

eqyiel · 2017-09-05T23:07:29Z

@FRidh was there something remaining to be done here?

eqyiel · 2017-09-07T23:55:30Z

Rebased!

This is a dependency of awsebcli.

In order to make it nicer to override when a specific version is required for Python derivations.

eqyiel · 2017-09-13T23:14:49Z

Rebased!

eqyiel · 2017-09-18T23:48:39Z

Bump. @FRidh is there something remaining to be done here?

eqyiel · 2017-09-23T00:48:04Z

@FRidh @edolstra ping

FRidh · 2017-09-23T06:25:14Z

I've pushed modified versions of these commits, e.g. b1c4498

FRidh · 2017-09-23T06:25:41Z

My apologies for the long delay.

eqyiel · 2017-09-24T02:59:05Z

@FRidh that's OK, thank you for your time!

eqyiel requested a review from FRidh as a code owner August 21, 2017 00:36

joachifm added 6.topic: python 8.has: package (new) labels Aug 26, 2017

eqyiel force-pushed the awsebcli branch from 92b8579 to c1e03fb Compare August 28, 2017 23:25

FRidh requested changes Aug 29, 2017

View reviewed changes

eqyiel force-pushed the awsebcli branch 2 times, most recently from 5e87a30 to e5222d6 Compare September 1, 2017 00:27

FRidh requested changes Sep 1, 2017

View reviewed changes

eqyiel force-pushed the awsebcli branch from e5222d6 to 76aaf37 Compare September 1, 2017 12:04

FRidh requested changes Sep 2, 2017

View reviewed changes

eqyiel force-pushed the awsebcli branch from 76aaf37 to f0f7e19 Compare September 7, 2017 23:54

eqyiel requested a review from edolstra as a code owner September 7, 2017 23:54

eqyiel added 6 commits September 14, 2017 08:44

maintainers.nix: Add myself

f0fa027

pythonPackages.blessed: init at 1.14.2

1951e8f

This is a dependency of awsebcli.

pythonPackages.cement: init at 2.8.2

b21900b

This is a dependency of awsebcli.

pythonPackages.tabulate: use fetchPypi

f6ebad0

In order to make it nicer to override when a specific version is required for Python derivations.

pythonPackages.semantic-version: use fetchPypi

bcdc77a

In order to make it nicer to override when a specific version is required for Python derivations.

awsebcli: init at 3.10.5

2ba4bc7

eqyiel force-pushed the awsebcli branch from f0f7e19 to 2ba4bc7 Compare September 13, 2017 23:14

FRidh closed this Sep 23, 2017

eqyiel deleted the awsebcli branch November 3, 2020 12:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

awsebcli: init at 3.10.5 #28428

awsebcli: init at 3.10.5 #28428

eqyiel commented Aug 21, 2017 •

edited

eqyiel commented Aug 28, 2017

FRidh left a comment

FRidh Aug 29, 2017

FRidh Aug 29, 2017

FRidh Aug 29, 2017

FRidh Aug 29, 2017

FRidh Aug 29, 2017

FRidh Aug 29, 2017

FRidh Aug 29, 2017

eqyiel commented Sep 1, 2017

FRidh left a comment

FRidh Sep 1, 2017

eqyiel Sep 1, 2017

FRidh Sep 1, 2017

eqyiel Sep 1, 2017

FRidh Sep 1, 2017

eqyiel Sep 1, 2017

FRidh Sep 2, 2017

eqyiel commented Sep 1, 2017

FRidh left a comment

FRidh Sep 2, 2017

eqyiel Sep 3, 2017

FRidh Sep 2, 2017

FRidh Sep 2, 2017

eqyiel Sep 3, 2017

eqyiel commented Sep 5, 2017

eqyiel commented Sep 7, 2017

eqyiel commented Sep 13, 2017

eqyiel commented Sep 18, 2017

eqyiel commented Sep 23, 2017

FRidh commented Sep 23, 2017

FRidh commented Sep 23, 2017

eqyiel commented Sep 24, 2017

awsebcli: init at 3.10.5 #28428

awsebcli: init at 3.10.5 #28428

Conversation

eqyiel commented Aug 21, 2017 • edited

Motivation for this change

Things done

eqyiel commented Aug 28, 2017

FRidh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eqyiel commented Sep 1, 2017

FRidh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eqyiel commented Sep 1, 2017

FRidh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eqyiel commented Sep 5, 2017

eqyiel commented Sep 7, 2017

eqyiel commented Sep 13, 2017

eqyiel commented Sep 18, 2017

eqyiel commented Sep 23, 2017

FRidh commented Sep 23, 2017

FRidh commented Sep 23, 2017

eqyiel commented Sep 24, 2017

eqyiel commented Aug 21, 2017 •

edited