paperless-ng: init at 1.4.5 #123046
paperless-ng: init at 1.4.5 #123046
Conversation
|
Result of 12 packages built successfully:
11 suggestions:
Result of 13 packages built successfully:
11 suggestions:
|
Good idea, I added an assertion if they both point to the same data directory. |
Both services still use the same port. We should change the default @Flakebi, could you address the |
|
Extended the assertion to check the port and fixed the warnings. |
There was a problem hiding this comment.
Most commit messages should start with pythonPackages or python3Packages. All Python packages should have explicit tests, i.e. set checkPhase or use pytestCheckHook (unless you set doCheck = false for some reason), and use pythonImportsCheck.
You might also want to look into doing hardening the systemd unit. Take inspiration from other modules or read https://www.freedesktop.org/software/systemd/man/systemd.exec.html#Sandboxing, https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html#IPAddressAllow=ADDRESS%5B/PREFIXLENGTH%5D%E2%80%A6, and https://www.freedesktop.org/software/systemd/man/systemd.resource-control.html#DevicePolicy=auto%7Cclosed%7Cstrict.
There was a problem hiding this comment.
I haven't yet finished my review, but to avoid duplications in @dotlambda's and my review, here are my fixes so far.
|
Thanks for the helpful reviews! @erikarvstedt, I added all the fixes from your branch. All new python packages now have |
There was a problem hiding this comment.
@Flakebi, thanks for packaging paperless-ng!
Here are some more fixups.
|
Thanks for the fixups @erikarvstedt! I kept the |
|
Just a thought: how about using services.paperless instead of services.paperless-ng? If I understood correctly the services are not intended to be run in parallel (same data directory, same port, etc. and more importantly same service). Even if it is possible, that would be a corner case I guess. Are people expected to use the old paperless? Are the modules interfaces similar? If the answers are no and yes, we could use the paperless-ng module+package if system.stateVersion is new enough. Just like a database engine update needing migration. |
|
Rebased to fix merge conflicts |
|
@ofborg test paperless-ng |
|
Thanks for working on this! Just a heads up, looks like the latest release is now 1.4.5 :) (edit: I just saw that you did update the version, PR title and commit still mention previous one) |
|
Rebased to fix merge conflicts and corrected commit message to 1.4.5. |
|
Do you think this could get backported to 21.05 once it gets merged, without the commit removing paperless? I'd be interested in running this on stable :) |
It may be that easy or it may be not. If we need to backport more things I don't think we should do it at all and generally I don't think we should backport new additions anymore. |
|
|
|
Rebased to fix merge conflicts and fix test failures caused by Pillow update. |
|
Rebased and slightly improved security by removing |
| checkPhase = '' | ||
| pushd src | ||
| PATH="${path}:$PATH" HOME=$(mktemp -d) XDG_DATA_DIRS="${liberation_ttf}/share:$XDG_DATA_DIRS" pytest | ||
| popd | ||
| ''; |
There was a problem hiding this comment.
This should use pytestCheckHook instead.
There was a problem hiding this comment.
As discussed before, pytestCheckHook introduces needless complexity by requiring an extra postCheck hook for restoring the working directory. Using checkPhase is shorter and clearer.
There was a problem hiding this comment.
As soon as we need to disable some test or directory of tests pytestCheckHook is way cleaner and easier to use. The extra postPhase does not matter. If we have hooks and functions we should use them instead of reimplementing stuff.
There was a problem hiding this comment.
@SuperSandro2000, I'd refer to YAGNI in this case and steer on the side of explicitness and simplicity.
@andir, would you consider merging this PR in its current state, given the merge conflict is resolved? I'd really love to see this getting finished.
There was a problem hiding this comment.
@erikarvstedt Yeah, I am looking forward to having this in nixpkgs so I can stop rebasing :)
|
I would also very much like to see this get merged! I've had a difficult time trying to incorporate this package into my NixOS. For what it's worth, I've tried to run nix build 'github:Flakebi/nixpkgs/32095dd69ca8af3fd55e2224b60e21a0ae074a8a#paperless-ng'And I get: [ntdef@nixos ~]$ nix build github:Flakebi/nixpkgs/32095dd69ca8af3fd55e2224b60e21a0ae074a8a#paperless-ng
error: builder for '/nix/store/y4nnyrqzf98i5qs1mlnmgwxycd1a8ik0-paperless-ng-1.4.5.drv' failed with exit code 1;
last 10 log lines:
> =========================== short test summary info ============================
> FAILED documents/tests/test_management_consumer.py::TestConsumerPolling::test_slow_write_pdf
> FAILED documents/tests/test_management_consumer.py::TestConsumerRecursivePolling::test_slow_write_pdf
>
> Results (97.77s):
> 427 passed
> 2 failed
> - documents/tests/test_management_consumer.py:134 TestConsumerPolling.test_slow_write_pdf
> - documents/tests/test_management_consumer.py:134 TestConsumerRecursivePolling.test_slow_write_pdf
> 2 skipped
For full logs, run 'nix log /nix/store/y4nnyrqzf98i5qs1mlnmgwxycd1a8ik0-paperless-ng-1.4.5.drv'.Here are the full logs: |
…less-ng The paperless project has moved on to paperless-ng and the original paperless package in Nixpkgs has stopped working recently (due to version incompatibility with the providede Django package). Instead of investing more time into the old module we should migrate all users to the new module instead.
|
@ofborg test paperless-ng |
|
@andir, the test succeeds. Is there anything left to be done? |
Motivation for this change
“A supercharged version of paperless: scan, index and archive all your physical documents”: https://github.com/jonaswinkler/paperless-ng
Activating
services.paperless-ng.enable = true;updates the database, so be sure to make a backup if you used paperless before (the services use the same default directory).Fixes #116761
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)