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

nixos/paperless-ngx: fix module 1.10.2 #207754

Merged

Conversation

leona-ya
Copy link
Member

@leona-ya leona-ya commented Dec 25, 2022

Description of changes

With the update to paperless-ngx to 1.10.2 (#206835) the task queue of paperless-ngx was changed to celery. This broke the NixOS module.

fixes #207965

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@leona-ya
Copy link
Member Author

Currently the NixOS test fails but I don't understand why. I tried this on my instance and it works...

@leona-ya leona-ya changed the title Paperless ngx/fix module 1.10.2 paperless-ngx/fix module 1.10.2 Dec 25, 2022
@leona-ya leona-ya changed the title paperless-ngx/fix module 1.10.2 nixos/paperless-ngx: fix module 1.10.2 Dec 25, 2022
@elohmeier
Copy link
Contributor

I think management of the NLTK data files should be added as well, see e.g. https://github.com/paperless-ngx/paperless-ngx/pull/2129/files for the upstream docker image changes they decided to do. We could fetch the NLTK data files using fetchzip and populate the PAPERLESS_NLTK_DIR env var accordingly.

@NobbZ
Copy link
Contributor

NobbZ commented Dec 27, 2022

master fails as well after 20 minutes… So as the test doesn't seem to block advancing the channels (we probably wouldn't have this problem if it would), we could as well remove it…

@leona-ya
Copy link
Member Author

@NobbZ the test fails on master because paperless-ngx is broken there. In #206835 the package was updated without the NixOS module update. This PR updates the NixOS module, so the test should work. Therefore the test should be fixed.

@leona-ya
Copy link
Member Author

@elohmeier Good idea. I would add that as an optional parameter later.

@NobbZ
Copy link
Contributor

NobbZ commented Dec 27, 2022

I am aware that paperless is broken there.

And as the tests are failing, channels shouldn't have advanced. Or we can remove the tests as irrelevant.

Thats what I wanted to say…

@leona-ya
Copy link
Member Author

Ah, sorry I misread your post. The NixOS test could have prevented merging the package update PR. Therefore I think the test is relevant in general. We could disable the test for now because it's more important to fix master (and could add NLTK support in another PR)

@SuperSandro2000
Copy link
Member

And as the tests are failing, channels shouldn't have advanced.

Only a subset of the tests block channel advancing. paperless should surely not be one of them.

@SuperSandro2000 SuperSandro2000 merged commit ad8ae1b into NixOS:master Dec 28, 2022
@NobbZ
Copy link
Contributor

NobbZ commented Dec 28, 2022

Which I totally understand, though why does the test exist at all then? It is obviously neither checked when merging, nor when advancing, so why take the burden of maintanance?

@SuperSandro2000
Copy link
Member

We should definitely add it to passthru and fix it.

@babariviere
Copy link
Member

Just seeing this PR. Sorry for the update of paperless-ngx. I should have checked that it would work with the module. Will be more careful next time. And thanks for fixing it!

@ambroisie
Copy link
Contributor

I think there is a missing dependency on redis-paperless.service for paperless-task-queue.service.

@leona-ya
Copy link
Member Author

leona-ya commented Jan 4, 2023

@ambroisie That should be okay i think. paperless-task-queue is started by paperless-scheduler and this service starts after redis-paperless

@leona-ya leona-ya deleted the paperless-ngx/fix-module-1.10.2 branch January 4, 2023 19:30
@ambroisie
Copy link
Contributor

@leona-ya that seems surprising, I had an issue due to the missing dependency when nixos-rebuild switching after updating my nixpkgs input.

Will have to see if it happens again in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

paperless: scheduler service can't start, "Unknown command: 'qcluster'"
6 participants