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

paperless: add package and service #44086

Merged
merged 9 commits into from May 8, 2019

Conversation

@erikarvstedt
Copy link
Contributor

commented Jul 25, 2018

Notes for reviewers

Paperless has no setup.py, the main way to use it is to call manage.py in a properly setup
Python environment. That's what ${paperless}/bin/paperless does.

Paperless requires some Python packages that are missing from nixpkgs. Currently, they are defined in extra-python-packages.nix. Should I add them all to python-packages.nix? Do we need an extra git commit for each package?

Here's a sample script for testing:

dataDir=/tmp/paperless-data
rm -rf $dataDir

nix-build --out-link ./paperless - <<EOF
(import <nixpkgs> {}).paperless.withConfig {
  dataDir = $dataDir;
  config = {
    PAPERLESS_DISABLE_LOGIN = "true";
  };
}
EOF

# Setup DB
./paperless migrate

# Create a test document
$(nix-build --no-out-link '<nixpkgs>' -A imagemagick)/bin/convert \
  -size 400x40 xc:white -font "DejaVu-Sans" -pointsize 20 -fill black -annotate +5+20 \
  "hello world 16-10-2005" $dataDir/consume/doc.png

# Consume document
./paperless document_consumer --oneshot

# Start webinterface
./paperless runserver --noreload localhost:8000

# Cleanup
rm -rf $dataDir ./paperless
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 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.

@jfrankenau

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2018

Python packages should be globally available. Nowadays each package gets one directory/file. One commit per package would be nice but it may be ok to put them all in one.

Have you thought about creating a matching NixOS module?

@dotlambda
Copy link
Member

left a comment

Please create the packages besides paperless itself inside the pythonPackages package set. Some of them might already be packaged.

@erikarvstedt

This comment has been minimized.

Copy link
Contributor Author

commented Aug 20, 2018

Back from vacation, please excuse the delay!

@dotlambda, done.

@jfrankenau, I've added a service module, but there are a few open issues:

  • The web interface (paperless-server) runs on the Django dev server. Should I use a proper web server like nginx or Apache?
  • By default, the services run with user nobody. Should I add a dedicated paperless user (via ids.nix)?
  • In a service preStart script im using ${pkgs.libuuid}/bin/runuser to run a cmd as the unprivileged user. Is there better, canonical way to do this? su is unavailable in the script env.

I'm using this script for testing the service.
Run this for a quick demo.

@erikarvstedt erikarvstedt changed the title paperless: init at 2.1.0 paperless: add package and service Aug 20, 2018

@jfrankenau

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2018

@erikarvstedt, I guess for now using Django is OK.

/var/lib/paperless should be owned by a real user, so adding a new user to ids.nix is necessary.

What about adding another ExecStart line to run the unprivileged migration process? Or even extract it to another systemd oneshot service?

nixos/modules/services/misc/paperless.nix Outdated

consumptionDir = mkOption {
type = with types; nullOr str;
default = null;

This comment has been minimized.

Copy link
@jfrankenau

jfrankenau Aug 21, 2018

Contributor

Set it to the default value mentioned in the description and remove the if-clause below.

This comment has been minimized.

Copy link
@erikarvstedt

erikarvstedt Aug 21, 2018

Author Contributor

Good catch! Fixed.

@erikarvstedt erikarvstedt force-pushed the erikarvstedt:paperless branch Aug 21, 2018

@erikarvstedt

This comment has been minimized.

Copy link
Contributor Author

commented Aug 21, 2018

The migration has to run in a preStart script so that the server, which is only active when the consumer is active, is disabled during migrations.
I've also thought about a helper oneshot service, but I think that's even more complicated.

A dedicated paperless user would be no more 'real' than the nobody user.
The only advantage would be isolation from other processes that might be running as the nobody user. But is this worth the extra effort? Is there relevant NixOS documentation?

@jfrankenau

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2018

Adding a new user is as you say for isolation purposes. Especially as /var/lib/paperless shouldn't be accessible to other users.

@erikarvstedt

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2018

Ok, done.

@erikarvstedt erikarvstedt force-pushed the erikarvstedt:paperless branch 2 times, most recently Aug 22, 2018

pkgs/top-level/python-packages.nix Outdated
@@ -284,7 +284,9 @@ in {

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

globus-sdk = callPackage ../development/python-modules/globus-sdk { };

This comment has been minimized.

Copy link
@globin

globin Aug 28, 2018

Member

You apparently accidentally deleted this line which causes the CI to fail during evaluation:
https://gist.github.com/GrahamcOfBorg/14241c580774a606e627f780e1809db9

This comment has been minimized.

Copy link
@erikarvstedt

erikarvstedt Aug 28, 2018

Author Contributor

Done. Thanks for digging into this, I thought that error came from upstream.

@erikarvstedt erikarvstedt force-pushed the erikarvstedt:paperless branch from 90298fd to 78bb5bc Feb 12, 2019

@erikarvstedt

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

78bb5bc: Rebased to master, squashed fixup commits.

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Apr 24, 2019

@dotlambda Do you feel this is suitable?

Don't think it's fair to @erikarvstedt to leave this unmerged any longer.

@erikarvstedt

This comment has been minimized.

Copy link
Contributor Author

commented Apr 25, 2019

I can understand if the review and maintanence burden of this feature seems unreasonable to you. I'd be fine with moving it to NUR then.

@worldofpeace

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

I can understand if the review and maintanence burden of this feature seems unreasonable to you. I'd be fine with moving it to NUR then.

Oh no, I think this is perfectly suitable to have in nixpkgs.

@dotlambda
Copy link
Member

left a comment

Only some really minor stuff. And yes, this should definitely be merged.

Show resolved Hide resolved nixos/modules/services/misc/paperless.nix Outdated
Show resolved Hide resolved pkgs/applications/office/paperless/default.nix Outdated
django_2_0 = pyPkgs: pyPkgs.django_2_1.overrideDerivation (_: rec {
pname = "Django";
version = "2.0.12";
name = "${pname}-${version}";

This comment has been minimized.

Copy link
@dotlambda

dotlambda Apr 26, 2019

Member
Suggested change
name = "${pname}-${version}";

This comment has been minimized.

Copy link
@erikarvstedt

erikarvstedt Apr 29, 2019

Author Contributor

For overrideDerivation , we have to update name manually.

set -e
${setupEnv}
${extraCmds}
exec python $paperlessSrc/manage.py "$@"

This comment has been minimized.

Copy link
@dotlambda

dotlambda Apr 26, 2019

Member
Suggested change
exec python $paperlessSrc/manage.py "$@"
exec ${paperless.python.interpreter} $paperlessSrc/manage.py "$@"

This comment has been minimized.

Copy link
@erikarvstedt

erikarvstedt Apr 29, 2019

Author Contributor

The correct form would be paperless.pythonEnv.interpreter.
setupEnv already handles all these details, so I'd rather not repeat them to keep it DRY.

erikarvstedt added some commits Jan 27, 2019

@erikarvstedt erikarvstedt force-pushed the erikarvstedt:paperless branch from 78bb5bc to 28b131f Apr 29, 2019

@erikarvstedt erikarvstedt force-pushed the erikarvstedt:paperless branch from 28b131f to b5651b3 Apr 29, 2019

@erikarvstedt

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

b5651b3: Rebased to master

  • Updated djangoql to the latest release
  • Provide django-filter, django-crispy-forms as private implementations, because
    we use unreleased versions and patches to make them compatible with the current nixpkgs
    python package env.

@erikarvstedt erikarvstedt force-pushed the erikarvstedt:paperless branch from d0ed820 to 505f6f3 Apr 29, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member

commented May 7, 2019

@erikarvstedt If you can rebase/squash this I can merge this.

erikarvstedt added some commits Jan 27, 2019

@erikarvstedt erikarvstedt force-pushed the erikarvstedt:paperless branch from 505f6f3 to 80c3ddb May 8, 2019

@worldofpeace worldofpeace merged commit bb7e556 into NixOS:master May 8, 2019

14 of 15 checks passed

paperless on aarch64-linux Failure
Details
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
paperless on x86_64-linux Success
Details
@worldofpeace

This comment has been minimized.

Copy link
Member

commented May 8, 2019

Thank you @erikarvstedt for your patience (also very organized summary of changes which helps with reviews) and of course your contributions

Likewise thanks to our reviewers 👍

@FRidh

This comment has been minimized.

Copy link
Member

commented May 9, 2019

The attribute name is the attribute in the package set, and not the pname or name attribute in the expression.

@c0bw3b c0bw3b referenced this pull request May 9, 2019

Open

Some python libraries, mostly scrapying-related #60775

2 of 10 tasks complete
@erikarvstedt

This comment has been minimized.

Copy link
Contributor Author

commented May 9, 2019

Huge thanks for your help and feedback, @dotlambda, @worldofpeace!

@FRidh, you meant to post that in another thread, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.