octoprint.python.pkgs.octolapse: init at 0.4.1 #136513

j0hax · 2021-09-02T21:56:03Z

Motivation for this change

Package the popular Octolapse plugin along with an additional python dependency

Things done

fabaff · 2021-09-05T08:31:36Z

pkgs/applications/misc/octoprint/plugins.nix

+    meta = with lib; {
+      description = "Stabilized timelapses for Octoprint";
+      homepage = "https://github.com/FormerLurker/OctoLapse";
+      license = licenses.agpl3;


According to the file headers (e.g., 1 and 2) the license is AGPL3+.

Suggested change

license = licenses.agpl3;

license = licenses.agpl3Plus;

fabaff · 2021-09-05T08:31:42Z

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

+{ lib, buildPythonPackage, fetchPypi, mock }:
+
+buildPythonPackage rec {
+  pname = "file_read_backwards";


Suggested change

pname = "file_read_backwards";

pname = "file-read-backwards";

Naming should follow PEP 0503 -> https://nixos.org/manual/nixpkgs/unstable/#contributing

j0hax · 2021-09-05T14:25:22Z

As #136349 was coincidentally opened very shortly before my PR, should I remove f567732?

illustris · 2021-09-05T20:06:16Z

It's better to have file-read-backwards added this way. Use this PR for octolapse, I'll drop it from mine.

j0hax · 2021-09-05T20:18:25Z

@illustris would you like me to add you as a maintainer for octolapse as well?

illustris · 2021-09-06T04:27:37Z

Sure. I actively use it, so I'd be happy to fix it if it breaks in the future.

nixos-discourse · 2021-09-18T07:26:54Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/595

pkgs/top-level/python-packages.nix

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

urbas · 2021-09-19T08:34:49Z

pkgs/applications/misc/octoprint/plugins.nix

+      sha256 = "13q20g7brabplc198jh67lk65rn140r8217iak9b2jy3in8fggv4";
+    };
+
+    propagatedBuildInputs = with super; [ awesome-slugify setuptools pillow sarge six psutil file_read_backwards ];


Suggested change

propagatedBuildInputs = with super; [ awesome-slugify setuptools pillow sarge six psutil file_read_backwards ];

propagatedBuildInputs = with super; [ awesome-slugify setuptools pillow sarge six psutil file-read-backwards ];

Thinking more about this, I wonder why this doesn't fail the build. Shouldn't these plugins be included in octoprint's build somehow? Maybe there should be an octoprint-full package in all-packages.nix or something like that?

Would you know how is one supposed to use these plugins or at least build them?

Edit: Okay, found a way to build this:

% nix build -Lvf . --no-link pkgs.octoprint.python.pkgs.octolapse error: undefined variable 'file_read_backwards' at /home/mat/programming/nixpkgs/pkgs/applications/misc/octoprint/plugins.nix:413:94 (use '--show-trace' to show detailed location information)

I would vote to add a octoprint-full into the top-level and make sure these plugins actually build.

A -full package with all optional plugins isn't a pattern I've seen often in nixpkgs. For example:

https://github.com/NixOS/nixpkgs/tree/master/pkgs/servers/monitoring/grafana
https://github.com/NixOS/nixpkgs/tree/master/pkgs/servers/roundcube
https://github.com/NixOS/nixpkgs/tree/master/pkgs/shells/fish
https://github.com/NixOS/nixpkgs/tree/master/pkgs/applications/networking/cluster/helm

Gimp has plugins, and has a wrapper package gimp-with-plugins, but even this defaults to plugins ? null
https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/graphics/gimp/wrapper.nix

Gimp differs from the above packages in having its plugins exposed as gimpPlugins.<name>. Yosys does something similar. This might be less confusing than having a -full package with all plugins, especially given some of these plugins are very situational and not needed by most people. The ender3v2tempfix plugin, for example, is essential if and only if you have an ender 3 v2 printer.

The name needs to be updated or we run into an eval error when the addon is used.

SuperSandro2000 · 2021-09-23T12:12:03Z

pkgs/development/python-modules/file-read-backwards/default.nix

+  version = "2.0.0";
+
+  src = fetchPypi {
+    pname = builtins.replaceStrings ["-"] ["_"] pname;


Use lib instead of builtins.

I remember someone saying something about lib enabling reflection, but I can't find it any more. @SuperSandro2000 can you point me to any documentation / discussion about why lib is preferred over builtins here?

There is no documentation for every little thing. lib sometimes has some extra sugar on top of the builtins like fetchurl and it is just easier to use lib everywhere because you don't need to think about anything. For replaceStrings lib and builtins are identical.

urbas

Successfully built nix build -Lvf . --no-link pkgs.octoprint.python.pkgs.octolapse. 👍

pkgs/development/python-modules/file-read-backwards/default.nix

pkgs/applications/misc/octoprint/plugins.nix

pkgs/development/python-modules/file-read-backwards/default.nix

urbas · 2021-11-27T16:02:30Z

pkgs/applications/misc/octoprint/plugins.nix

+      sha256 = "13q20g7brabplc198jh67lk65rn140r8217iak9b2jy3in8fggv4";
+    };
+
+    pythonImportsCheck = [ "file_read_backwards" ];


TL;DR: I think we'll have to give up on the import check in this plugin. I suggest you remove this import check entirely (read below for a more detailed explanation):

Suggested change

pythonImportsCheck = [ "file_read_backwards" ];

Detailed explanation:

Similarly to the other comment, this should be like this:

pythonImportsCheck = [ "octoprint_octolapse" ];

However, I just checked and octoprint's plugins execute some code on import which fails the import check:

python3.8-OctoPrintPlugin-Octolapse> Check whether the following modules can be imported: octoprint_octolapse python3.8-OctoPrintPlugin-Octolapse> Initializing GcodePositionProcessor V1.0.1 - Copyright (C) 2019 Brad Hochgesang...Python 3+ Detected...complete python3.8-OctoPrintPlugin-Octolapse> 2021-11-27 16:07:30,253 - octolapse.__init__ - INFO - Release mode detected. python3.8-OctoPrintPlugin-Octolapse> Error in atexit._run_exitfuncs: python3.8-OctoPrintPlugin-Octolapse> Traceback (most recent call last): python3.8-OctoPrintPlugin-Octolapse> File "/nix/store/66p7i56lg3zzp7nj6g0fywb704ihp4iy-python3-3.8.11/lib/python3.8/logging/__init__.py", line 2127, in shutdown python3.8-OctoPrintPlugin-Octolapse> h.close() python3.8-OctoPrintPlugin-Octolapse> File "/nix/store/iqdafs15896fzih94jgicr7lgx0b4h5j-python3.8-OctoPrint-1.6.1/lib/python3.8/site-packages/octoprint/logging/handlers.py", line 31, in close python3.8-OctoPrintPlugin-Octolapse> self._executor.shutdown(wait=True) python3.8-OctoPrintPlugin-Octolapse> AttributeError: 'OctolapseConsoleHandler' object has no attribute '_executor'

@SuperSandro2000: I suggest we omit the imports check here. It would require a change a change to the buildPlugin function to support this.

Makes sense to me. If everything else looks good, I'll go ahead and comment out the check, it may be sensible to add it at a later time when the import works.

Good to know.

j0hax · 2022-01-16T16:28:53Z

Anything else needed in order to finish this PR up?

SuperSandro2000 · 2022-01-16T22:35:57Z

rebase and force push to fix the merge conflict.

SuperSandro2000 · 2022-01-19T11:12:16Z

This is a semi-automatic executed nixpkgs-review with nixpkgs-review-checks extension. It is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).

Result of nixpkgs-review pr 136513 run on x86_64-linux 1

2 packages built:

python310Packages.file-read-backwards
python39Packages.file-read-backwards

j0hax requested review from FRidh and jonringer as code owners September 2, 2021 21:56

github-actions bot added the 6.topic: python label Sep 2, 2021

ofborg bot added 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Sep 2, 2021

fabaff reviewed Sep 5, 2021

View reviewed changes

fabaff mentioned this pull request Sep 5, 2021

octoprint.python.pkgs.ender3v2tempfix: init at unstable-2021-04-27 #136349

Merged

12 tasks

j0hax requested a review from fabaff September 6, 2021 14:48

urbas reviewed Sep 18, 2021

View reviewed changes

pkgs/top-level/python-packages.nix Outdated Show resolved Hide resolved

urbas reviewed Sep 18, 2021

View reviewed changes

pkgs/development/python-modules/file_read_backwards/default.nix Outdated Show resolved Hide resolved

urbas reviewed Sep 19, 2021

View reviewed changes

SuperSandro2000 reviewed Sep 23, 2021

View reviewed changes

urbas approved these changes Nov 7, 2021

View reviewed changes

SuperSandro2000 reviewed Nov 9, 2021

View reviewed changes

pkgs/development/python-modules/file-read-backwards/default.nix Outdated Show resolved Hide resolved

SuperSandro2000 reviewed Nov 9, 2021

View reviewed changes

pkgs/applications/misc/octoprint/plugins.nix Show resolved Hide resolved

SuperSandro2000 reviewed Nov 25, 2021

View reviewed changes

pkgs/development/python-modules/file-read-backwards/default.nix Show resolved Hide resolved

urbas suggested changes Nov 27, 2021

View reviewed changes

ofborg bot added the 2.status: merge conflict label Nov 28, 2021

j0hax added 2 commits January 18, 2022 20:51

octoprint.python.pkgs.octolapse: init at 0.4.1

a85dc0c

python3Packages.file_read_backwards: init at 2.0.0

2fe8763

ofborg bot removed the 2.status: merge conflict label Jan 18, 2022

SuperSandro2000 merged commit 9693f55 into NixOS:master Jan 19, 2022

j0hax deleted the octolapse branch January 19, 2022 12:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

octoprint.python.pkgs.octolapse: init at 0.4.1 #136513

octoprint.python.pkgs.octolapse: init at 0.4.1 #136513

j0hax commented Sep 2, 2021

fabaff Sep 5, 2021

fabaff Sep 5, 2021

j0hax commented Sep 5, 2021

illustris commented Sep 5, 2021

j0hax commented Sep 5, 2021

illustris commented Sep 6, 2021

nixos-discourse commented Sep 18, 2021

urbas Sep 19, 2021 •

edited

illustris Sep 19, 2021 •

edited

SuperSandro2000 Oct 3, 2021

SuperSandro2000 Sep 23, 2021

ilkecan Sep 24, 2021

illustris Sep 26, 2021

SuperSandro2000 Oct 3, 2021

urbas left a comment

urbas Nov 27, 2021

j0hax Nov 28, 2021

SuperSandro2000 Nov 29, 2021

j0hax commented Jan 16, 2022

SuperSandro2000 commented Jan 16, 2022

SuperSandro2000 commented Jan 19, 2022

	pname = "file_read_backwards";
	pname = "file-read-backwards";

	propagatedBuildInputs = with super; [ awesome-slugify setuptools pillow sarge six psutil file_read_backwards ];
	propagatedBuildInputs = with super; [ awesome-slugify setuptools pillow sarge six psutil file-read-backwards ];

octoprint.python.pkgs.octolapse: init at 0.4.1 #136513

octoprint.python.pkgs.octolapse: init at 0.4.1 #136513

Conversation

j0hax commented Sep 2, 2021

Motivation for this change

Things done

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j0hax commented Sep 5, 2021

illustris commented Sep 5, 2021

j0hax commented Sep 5, 2021

illustris commented Sep 6, 2021

nixos-discourse commented Sep 18, 2021

urbas Sep 19, 2021 • edited

Choose a reason for hiding this comment

illustris Sep 19, 2021 • edited

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

urbas 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

j0hax commented Jan 16, 2022

SuperSandro2000 commented Jan 16, 2022

SuperSandro2000 commented Jan 19, 2022

urbas Sep 19, 2021 •

edited

illustris Sep 19, 2021 •

edited