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

etebase-server: add option withPostgres #196342

Merged
merged 1 commit into from Nov 5, 2022

Conversation

duament
Copy link
Contributor

@duament duament commented Oct 17, 2022

Description of changes

On current nixos-unstable branch, the following etebase-server config using postgresql backend

services.etebase-server = {
  enable = true;
  settings = {
    global.secret_file = config.sops.secrets."etebase/secret".path;
    allowed_hosts.allowed_host1 = "example.com";
    database = {
      engine = "django.db.backends.postgresql";
      name = "etesync";
      user = "etesync";
      host = "postgres.example.com";
      port = 5432;
    };
    database-options.passfile = config.sops.secrets."etebase/postgresql".path;
  };
};

fails with

Traceback (most recent call last):
  File "/nix/store/1bd23bjb8kirg6iz59j35155724qp2hk-python3.10-django-3.2.15/lib/python3.10/site-packages/django/db/backends/postgresql/base.py", line 25, in <module>
    import psycopg2 as Database
ModuleNotFoundError: No module named 'psycopg2'
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/)
  • 22.11 Release Notes (or backporting 22.05 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.

@duament
Copy link
Contributor Author

duament commented Oct 17, 2022

@felschr

Copy link
Member

@RaitoBezarius RaitoBezarius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@felschr felschr self-requested a review October 26, 2022 16:55
Copy link
Member

@felschr felschr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we also have an option for the package instead of adding it only in the module?
It could be done in the same way as the withLdap option with the default being true.

@duament
Copy link
Contributor Author

duament commented Nov 2, 2022

@felschr Added a withPostgres option. I wonder if we still need the patch in the module?

pkgs/servers/etebase/default.nix Outdated Show resolved Hide resolved
Comment on lines 9 to 11
[ etebase-server daphne ]
++ optional (cfg.settings.database.engine == "django.db.backends.postgresql") psycopg2
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove this since withPostgres's default is true.
But I guess we could also set withPostgres to false by default and keep this.
Would be nice to have a third opinion on this.

@ofborg ofborg bot requested a review from felschr November 3, 2022 14:52
@duament duament changed the title nixos/etebase-server: add missing dependency psycopg2 for postgresql backend etebase-server: add missing dependency psycopg2 for postgresql backend Nov 3, 2022
Copy link
Member

@felschr felschr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you turn this into a single commit? Otherwise, I think it's good to merge.

@duament duament changed the title etebase-server: add missing dependency psycopg2 for postgresql backend etebase-server: add option withPostgres Nov 4, 2022
@ofborg ofborg bot requested a review from felschr November 4, 2022 03:29
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/667

@SuperSandro2000 SuperSandro2000 merged commit f8152fb into NixOS:master Nov 5, 2022
@duament duament deleted the etebase-postgresql branch November 25, 2022 07:21
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.

None yet

6 participants