Skip to content

nixos/pgadmin4: init#161521

Merged
mkg20001 merged 4 commits intoNixOS:masterfrom
mkg20001:pgadmin-mod
Feb 26, 2022
Merged

nixos/pgadmin4: init#161521
mkg20001 merged 4 commits intoNixOS:masterfrom
mkg20001:pgadmin-mod

Conversation

@mkg20001
Copy link
Member

Motivation for this change

This adds a module for pgadmin4

NOTE: depends on #154764

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.05 Release Notes (or backporting 21.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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 23, 2022
@mkg20001 mkg20001 force-pushed the pgadmin-mod branch 2 times, most recently from 1ce0d88 to 1516870 Compare February 23, 2022 13:47
@mkg20001 mkg20001 mentioned this pull request Feb 23, 2022
13 tasks
@ofborg ofborg bot added 8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package labels Feb 23, 2022
@ofborg ofborg bot requested review from domenkozar, gador and wmertens February 23, 2022 13:52
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 23, 2022
Copy link
Member

@gador gador left a comment

Choose a reason for hiding this comment

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

Thanks for the writeup of the module!
I added some comments and would suggest you add options for the state dir and log dir. Possibly you should consider adding an option to use Desktop Mode (which would enable the user to use initial passwords and emails), although I'd personally prefer to only support Server Edition.

Copy link
Member

Choose a reason for hiding this comment

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

I'd remove that

@mkg20001
Copy link
Member Author

mkg20001 commented Feb 23, 2022

@gador Desktop mode sounds like something that should go under programs.pgadmin4, but I'm not sure how much configuration (if any) it requires. To me it seems like mostly adding pgadmin4 to systemPackages.

Edit: or maybe should even go into home-manager, as it would likely be something specific to each user.

@ofborg ofborg bot requested a review from gador February 23, 2022 16:29
@mkg20001 mkg20001 force-pushed the pgadmin-mod branch 2 times, most recently from 35a2604 to 4c25b44 Compare February 23, 2022 20:53
@mkg20001
Copy link
Member Author

Does it make sense to split out the python config generator into it's own type in pkgs/build-support/formats.nix?

@gador
Copy link
Member

gador commented Feb 26, 2022

Does it make sense to split out the python config generator into it's own type in pkgs/build-support/formats.nix?

Do you mean pkgs/pkgs-lib/formats.nix?
I don't believe so. Looking at the directory /var/lib/pgadmin I only see a pgadmin4.db file (a sqlite file), which contains the config.
Even if we add a config-to-sql generator to formats.nix: We'd still add the clear password to the service-configuration. Also the config file includes a whole lot more than just the user and (hashed) password.

I think it makes sense to let pgadmin create its own config database. Can we make sure the user runs it (interactively) the first time?

@mkg20001
Copy link
Member Author

@gador Running interactivly imo is not an option. Similar to archisteamfarm, they also don't allow interactive stuff afaik.

As for concerns with putting stuff into the store: Instead a password file could be specified which would get read by the service on first startup, similar to e.g. discourse services.discourse.admin.passwordFile

Also the config that I'm talking about is /etc/pgadmin/config_system.py, which contains stuff like the port and oauth2 config, among others, not the config database, which is something different.

@mkg20001
Copy link
Member Author

bikeshed: pgadmin4 or pgadmin as module name?

@gador
Copy link
Member

gador commented Feb 26, 2022

I like the idea about the passwordFile. Its a way better option.

Also the config that I'm talking about is /etc/pgadmin/config_system.py, which contains stuff like the port and oauth2 config, among others, not the config database, which is something different.

Ok, that makes more sense. I don't mind the conversion logic within the module. Putting it in formats.nix seems to be a bit overkill..

It still does not solve the problem about the initial password, which gets asked on first run interactively.

@gador
Copy link
Member

gador commented Feb 26, 2022

bikeshed: pgadmin4 or pgadmin as module name?

I'd suggest pgadmin.

pgadmin3 doesn't need a module, and we already aliased pgadmin --> pgadmin4

EDIT:
Nevermind, we didn't alias. I'd still use pgadmin. Maybe we should alias 🤷

@mkg20001
Copy link
Member Author

@gador The interactive part is already solved by the preStart script

@gador
Copy link
Member

gador commented Feb 26, 2022

Ok. Then we just have to add the logic of a passwordFile to it?

I just noticed the use of pgadmin4-setup..Where does that binary come from? When I build pgadmin4, I just have pgadmin4 as a binary.

@mkg20001
Copy link
Member Author

@gador I made a patch d3a4625 (#161521)

This exposes setup.py as proper binary

@mkg20001 mkg20001 force-pushed the pgadmin-mod branch 2 times, most recently from 5ce52cb to ec802fe Compare February 26, 2022 12:55
@mkg20001 mkg20001 force-pushed the pgadmin-mod branch 2 times, most recently from 27aa7b0 to e343b98 Compare February 26, 2022 13:04
@ofborg ofborg bot added the 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. label Feb 26, 2022
@github-actions github-actions bot added 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation labels Feb 26, 2022
Copy link
Contributor

@wmertens wmertens left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@mkg20001 mkg20001 merged commit 7fbcb21 into NixOS:master Feb 26, 2022
@mkg20001 mkg20001 deleted the pgadmin-mod branch February 26, 2022 13:47
@gador gador mentioned this pull request Aug 28, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants