graphite: 1.0.2 -> 1.1.3 & moved dependencies to python-modules #44276

basvandijk · 2018-07-31T21:18:00Z

Motivation for this change

Fixes #30891 and closes #44246.

Things done

Upgrade graphite-web, carbon and whisper from 1.0.2 -> 1.1.3.
Extend the pythonPackages.callPackage scope with the
sharedVersions attribute set. This set contains versions that are
shared by multiple python packages. Currently it only contains the
graphiteVersion.
Replaced the deprecated pythonPackages.graphite_influxdb with
pythonPackages.influxgraph.
Renamed pythonPackages.graphite_web to pythonPackages.graphite-web
to be consistent with the Python package name.
Replaced the unmaintained pythonPackages.graphite_pager with
pythonPackages.graphitepager
Moved all new packages from python-packages.nix to
pkgs/development/python-modules
The following test passes:
nix-build nixos/release.nix -A tests.graphite.x86_64-linux

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 nox --run "nox-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)
Fits CONTRIBUTING.md.

@dotlambda @FRidh @adisbladis @offlinehacker @rickynils

dotlambda · 2018-07-31T21:29:53Z

What's the advantage of having graphiteVersion instead of specifying the version for each package? Do they really need to be the same version? E.g., graphite_api seems to work even though it is at a different version.
I'm asking because specifying the version individually would allow us to include the graphite packages in batch upgrades.

Mic92 · 2018-07-31T23:10:05Z

@GrahamcOfBorg test graphite

GrahamcOfBorg · 2018-07-31T23:10:44Z

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: tests.graphite

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnfree = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnfree = true; }
to ~/.config/nixpkgs/config.nix.

GrahamcOfBorg · 2018-07-31T23:13:17Z

Success on x86_64-linux (full log)

Attempted: tests.graphite

Partial log (click to expand)

one: exit status 1
syncing
one: running command: sync
one: exit status 0
test script finished in 26.38s
cleaning up
killing one (pid 597)
vde_switch: EOF on stdin, cleaning up and exiting
vde_switch: Could not remove ctl dir '/build/vde1.ctl': Directory not empty
/nix/store/98ai277yc66vbw89ks6akskcjavn5a0h-vm-test-run-graphite

basvandijk · 2018-07-31T23:53:09Z

@dotlambda it could be possible that different versions of graphite-web, carbon and whisper can work together. However, since they share the same version, are published on the same date and share the same release notes I think upstream intends them to be used together. Having a shared graphiteVersion ensures this in nixpkgs.

Note that we do something similar with the Elasticsearch stack.

Having support for batch upgrades would also be nice of course. I guess batch upgrades can also ensure we upgrade all graphite packages together. What needs to be done to get the graphite packages to batch upgrade?

FRidh · 2018-08-01T04:58:26Z

pkgs/top-level/python-packages.nix

+  # Some Python packages are released together and share the same version.
+  # These versions are collected in the sharedVersions attribute set and are
+  # added to the callPackage scope so that packages can refer to these versions.
+  sharedVersions = {


I like the idea, but as there currently is only one version I don't think we should have this. If we were to have this, I think this should go to top-level so we could also use it with say mercurial and tortoisehg and other cases there.

Another example: libgit2 and pygit2.

dotlambda · 2018-08-01T06:33:05Z

What needs to be done to get the graphite packages to batch upgrade?

Currently, our script can only deal with the version being specified in each file. If new versions are published on the same date, they would be updated simultaneously anyway.

basvandijk · 2018-08-01T06:43:07Z

@dotlambda then I don't mind duplicating the versions in each file.

BTW which script are you talking about? Somewhere in nixpkgs? If so, I could add the graphite packages to it.

dotlambda · 2018-08-01T06:45:59Z

There's no need to add packages manually. It's applied to all packages in pkgs/development/python-modules. You'll find it here: https://github.com/NixOS/nixpkgs/blob/master/maintainers/scripts/update-python-libraries

basvandijk · 2018-08-01T08:56:57Z

Great. I've updated my PR and removed graphiteVersion.

dotlambda · 2018-08-01T09:07:40Z

pkgs/development/python-modules/graphite-web/default.nix

+, whisper, pycairo, cairocffi, ldap, memcached, pytz, urllib3, scandir
+}:
+if django         != django_1_8
+|| django_tagging != django_tagging_0_4_3


Can we instead use something like django.version != "1.8"?

dotlambda · 2018-08-01T09:08:24Z

pkgs/development/python-modules/graphite-web/default.nix

+
+  postInstall = ''
+    wrapProgram $out/bin/run-graphite-devel-server.py \
+      --prefix PATH : ${which}/bin


You can use makeWrapperArgs instead.

I tried the following:

makeWrapperArgs = [ "--prefix PATH : ${which}/bin" ];

but with that the test suite fails to start up graphite-web:

one# [ 7.566444] graphiteWeb-pre-start[706]: Traceback (most recent call last): one# [ 7.568265] graphiteWeb-pre-start[706]: File "/nix/store/k8xndnycd95iqs0ky9nlcvqqnyx9p63v-python2.7-Django-1.8.18/bin/..django-admin.py-wrapped-wrapped", line 7, in <module> one# [ 7.574263] graphiteWeb-pre-start[706]: management.execute_from_command_line() one# [ 7.582220] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/django/core/management/__init__.py", line 354, in execute_from_command_line one# [ 7.595708] graphiteWeb-pre-start[706]: utility.execute() one# [ 7.602375] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/django/core/management/__init__.py", line 328, in execute one# [ 7.611485] graphiteWeb-pre-start[706]: django.setup() one# [ 7.617331] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/django/__init__.py", line 18, in setup one# [ 7.626695] graphiteWeb-pre-start[706]: apps.populate(settings.INSTALLED_APPS) one# [ 7.635331] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/django/apps/registry.py", line 85, in populate one# [ 7.646523] graphiteWeb-pre-start[706]: app_config = AppConfig.create(entry) one# [ 7.652897] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/django/apps/config.py", line 86, in create one# [ 7.665890] graphiteWeb-pre-start[706]: module = import_module(entry) one# [ 7.674238] graphiteWeb-pre-start[706]: File "/nix/store/7sbvn0wgv7hsnxpss1jba723kx5nz6d4-python-2.7.15/lib/python2.7/importlib/__init__.py", line 37, in import_module one# [ 7.682407] graphiteWeb-pre-start[706]: __import__(name) one# [ 7.693954] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/opt/graphite/webapp/graphite/functions/__init__.py", line 10, in <module> one# [ 7.703974] graphiteWeb-pre-start[706]: from graphite.logger import log one# [ 7.710845] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/opt/graphite/webapp/graphite/logger.py", line 106, in <module> one# [ 7.720270] graphiteWeb-pre-start[706]: log = GraphiteLogger() # import-shared logger instance one# [ 7.725289] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/opt/graphite/webapp/graphite/logger.py", line 39, in __init__ one# [ 7.732729] graphiteWeb-pre-start[706]: level = logging.DEBUG if settings.DEBUG else logging.INFO, one# [ 7.736779] graphiteWeb-pre-start[706]: File "/nix/store/h5pfcf6j3rl85xm4apggqg1d4za4yj53-python-2.7.15-env/lib/python2.7/site-packages/opt/graphite/webapp/graphite/logger.py", line 78, in _config_logger one# [ 7.743253] graphiteWeb-pre-start[706]: handler = Rotater(log_file, when=when, backupCount=backupCount) one# [ 7.748248] graphiteWeb-pre-start[706]: File "/nix/store/7sbvn0wgv7hsnxpss1jba723kx5nz6d4-python-2.7.15/lib/python2.7/logging/handlers.py", line 171, in __init__ one# [ 7.754796] graphiteWeb-pre-start[706]: BaseRotatingHandler.__init__(self, filename, 'a', encoding, delay) one# [ 7.762192] graphiteWeb-pre-start[706]: File "/nix/store/7sbvn0wgv7hsnxpss1jba723kx5nz6d4-python-2.7.15/lib/python2.7/logging/handlers.py", line 64, in __init__ one# [ 7.767696] graphiteWeb-pre-start[706]: logging.FileHandler.__init__(self, filename, mode, encoding, delay) one# [ 7.771913] graphiteWeb-pre-start[706]: File "/nix/store/7sbvn0wgv7hsnxpss1jba723kx5nz6d4-python-2.7.15/lib/python2.7/logging/__init__.py", line 920, in __init__ one# [ 7.779395] graphiteWeb-pre-start[706]: StreamHandler.__init__(self, self._open()) one# [ 7.784128] graphiteWeb-pre-start[706]: File "/nix/store/7sbvn0wgv7hsnxpss1jba723kx5nz6d4-python-2.7.15/lib/python2.7/logging/__init__.py", line 950, in _open one# [ 7.789363] graphiteWeb-pre-start[706]: stream = open(self.baseFilename, self.mode) one# [ 7.794295] graphiteWeb-pre-start[706]: IOError: [Errno 2] No such file or directory: '/var/db/graphite/log/webapp/info.log'

dotlambda · 2018-08-01T09:08:47Z

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

+
+  propagatedBuildInputs = [ tornado pyyaml funcparserlib ];
+
+  patchPhase = "> requirements.txt";


Why is this needed?

This was in the original package and without it the build fails at the dependency checking phase. I do feel a bit uneasy about just ignoring dependency version ranges.

How about using

substituteInPlace requirements.txt "==" ">="

instead?
Also, this should be put into postPatch.

Thanks, that's better indeed.

dotlambda · 2018-08-01T09:09:12Z

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

+    pyyaml redis requests six websocket_client
+  ];
+
+  patchPhase = "> requirements.txt";


Here as well. Would be best to get rid of it.

basvandijk · 2018-08-02T11:27:38Z

@dotlambda @FRidh I think this is ready to be merged now.

I also performed a manual test by modifying the graphite test to enable an xserver and install firefox. Then I ran the VM using $(nix-build nixos/tests/graphite.nix -A driver)/bin/nixos-run-vms and pointed firefox to graphite-web on localhost:8080. I could successfully operate the app.

FRidh · 2018-08-02T13:25:58Z

Unrelated, but:

  carbonEnv = {
    PYTHONPATH = let
      cenv = pkgs.python.buildEnv.override {
        extraLibs = [ pkgs.python27Packages.carbon ];
      };
      cenvPack =  "${cenv}/${pkgs.python.sitePackages}";
    # opt/graphite/lib contains twisted.plugins.carbon-cache
    in "${cenvPack}/opt/graphite/lib:${cenvPack}";

How about moving opt/graphite/lib to a Python package?

dotlambda · 2018-08-02T13:41:31Z

pkgs/development/python-modules/graphite-web/default.nix

+    homepage = http://graphite.wikidot.com/;
+    description = "Enterprise scalable realtime graphing";
+    maintainers = with stdenv.lib.maintainers; [ rickynils offline basvandijk ];
+  };


dotlambda · 2018-08-02T13:42:38Z

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

+    description = "InfluxDB storage plugin for Graphite-API";
+    homepage = https://github.com/InfluxGraph/influxgraph;
+    license = licenses.asl20;
+    platforms = platforms.linux;


I think this can be dropped. Or is it needed?

Yes, I think this can be dropped. I'll do it in a sec.

dotlambda · 2018-08-02T13:42:47Z

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

+    homepage = http://graphite.wikidot.com/;
+    description = "Fixed size round-robin style database";
+    maintainers = with stdenv.lib.maintainers; [ rickynils offline basvandijk ];
+  };


Fixes NixOS#30891 * Upgrade `graphite-web`, `carbon` and `whisper` from 1.0.2 -> 1.1.3. * Replaced the deprecated `pythonPackages.graphite_influxdb` with `pythonPackages.influxgraph.` * Renamed `pythonPackages.graphite_web` to `pythonPackages.graphite-web` to be consistent with the Python package name. * Replaced the unmaintained `pythonPackages.graphite_pager` with `pythonPackages.graphitepager` * Moved all new packages from `python-packages.nix` to `pkgs/development/python-modules`

dotlambda · 2018-08-02T14:40:26Z

Btw, should the changes to the module be mentioned in the changelog?

basvandijk · 2018-08-02T15:16:38Z

@dotlambda thanks for the merge. The API of the graphite module hasn't changed. So existing invocations of the module don't have to be changed. So I don't think a changelog entry is needed.

basvandijk · 2018-08-02T15:19:37Z

@FRidh about making $(nix-build -A python27Packages.carbon)/lib/python2.7/site-packages/opt/graphite/lib/ a Python package: this should ideally be done by upstream right?

basvandijk requested a review from FRidh as a code owner July 31, 2018 21:18

basvandijk mentioned this pull request Jul 31, 2018

pythonPackages: remove unmaintained graphite-related stuff #44246

Closed

9 tasks

GrahamcOfBorg added 6.topic: nixos 6.topic: python 8.has: module (update) labels Jul 31, 2018

GrahamcOfBorg added 8.has: clean-up 8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Jul 31, 2018

FRidh reviewed Aug 1, 2018

View reviewed changes

basvandijk force-pushed the graphite-1.1.3 branch from 0aeefe6 to 6fdaf30 Compare August 1, 2018 08:56

dotlambda reviewed Aug 1, 2018

View reviewed changes

basvandijk force-pushed the graphite-1.1.3 branch 2 times, most recently from 804e4b3 to 98f2009 Compare August 1, 2018 15:51

FRidh approved these changes Aug 2, 2018

View reviewed changes

dotlambda reviewed Aug 2, 2018

View reviewed changes

basvandijk force-pushed the graphite-1.1.3 branch from 98f2009 to f4e759b Compare August 2, 2018 14:23

dotlambda merged commit 0aae3fd into NixOS:master Aug 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

graphite: 1.0.2 -> 1.1.3 & moved dependencies to python-modules #44276

graphite: 1.0.2 -> 1.1.3 & moved dependencies to python-modules #44276

basvandijk commented Jul 31, 2018 •

edited by dotlambda

dotlambda commented Jul 31, 2018

Mic92 commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

basvandijk commented Jul 31, 2018

FRidh Aug 1, 2018

dotlambda Aug 1, 2018

dotlambda commented Aug 1, 2018

basvandijk commented Aug 1, 2018

dotlambda commented Aug 1, 2018

basvandijk commented Aug 1, 2018

dotlambda Aug 1, 2018

dotlambda Aug 1, 2018

basvandijk Aug 1, 2018

dotlambda Aug 1, 2018

basvandijk Aug 1, 2018

dotlambda Aug 1, 2018

basvandijk Aug 1, 2018

dotlambda Aug 1, 2018

basvandijk commented Aug 2, 2018

FRidh commented Aug 2, 2018

dotlambda Aug 2, 2018

dotlambda Aug 2, 2018

basvandijk Aug 2, 2018

dotlambda Aug 2, 2018

dotlambda commented Aug 2, 2018

basvandijk commented Aug 2, 2018

basvandijk commented Aug 2, 2018


		propagatedBuildInputs = [ tornado pyyaml funcparserlib ];

		patchPhase = "> requirements.txt";

graphite: 1.0.2 -> 1.1.3 & moved dependencies to python-modules #44276

graphite: 1.0.2 -> 1.1.3 & moved dependencies to python-modules #44276

Conversation

basvandijk commented Jul 31, 2018 • edited by dotlambda

Motivation for this change

Things done

dotlambda commented Jul 31, 2018

Mic92 commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

GrahamcOfBorg commented Jul 31, 2018

basvandijk commented Jul 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dotlambda commented Aug 1, 2018

basvandijk commented Aug 1, 2018

dotlambda commented Aug 1, 2018

basvandijk commented Aug 1, 2018

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

basvandijk commented Aug 2, 2018

FRidh commented Aug 2, 2018

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

dotlambda commented Aug 2, 2018

basvandijk commented Aug 2, 2018

basvandijk commented Aug 2, 2018

basvandijk commented Jul 31, 2018 •

edited by dotlambda