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.

Sign up for GitHub

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

Jump to bottom

Using psql command instead of postgres in initialScript #66

Merged
merged 19 commits into from
Dec 24, 2023
Merged

Using psql command instead of postgres in initialScript #66

merged 19 commits into from
Dec 24, 2023

Conversation

rsrohitsingh682
Copy link
Member

@rsrohitsingh682 rsrohitsingh682 commented Dec 20, 2023 •
edited by shivaraj-bh
Loading

Resolves : #63

Key changes in this PR:

  • Use psql instead of postgres, as a result use pg_ctl to temporarily start the postgres server in ${name}-init process.
  • Avoid creating a custom postgresPkg with extensions by using apply attribute provided by mkOption
  • Refactor setup script, use writeShellApplication and resolve shell-check errors

@rsrohitsingh682
Replaced postgres command with psql command and added dependOn for po…
6ce89ae 
…stgres server to start first then load initScript
@shivaraj-bh
Copy link
Member

The CI is going into an infinite loop while running the postgres_test.nix, stopping it for now.

@rsrohitsingh682
Run initdb before doing readlink
758fdad
@rsrohitsingh682
depend on <name>-init process instead of <name>
cf2a557 
Earlier <name> would depend on <name>-init, now it is the other way round
srid
srid reviewed Dec 21, 2023
View reviewed changes
example/flake.nix Outdated Show resolved Hide resolved
@shivaraj-bh shivaraj-bh mentioned this pull request Dec 22, 2023
Merged
@shivaraj-bh
create postgres/setup-script.nix, move setupIntialDatabases to it
1ffb2a7
@shivaraj-bh
move setupScript shell script
9dd8335
@shivaraj-bh
use pg_ctl to initialize database; run <name>-init before <name>
7f23b83 
By starting the server temporariliy using pg_ctl we can run the init process
before starting the server. Hence, the <name>-init needn't depend on <name>
anymore

The idea for pg_ctl comes from: https://github.com/cachix/devenv/blob/main/src/modules/services/postgres.nix
@shivaraj-bh
depend on <name> in tests and example
9425b1b
@shivaraj-bh
use writeShellApplication for setupScript
6bd6914 
get shell-check and be able to provide runtimeInputs (we will use this later)
@shivaraj-bh
shell-check: Declare and assign separately to avoid masking return va…
306b5b3 
…lues.
@shivaraj-bh
shell-check: Use single quotes, otherwise this expands now rather tha…
54cd92b 
…n when signalled.
@shivaraj-bh
shell-check: Use find instead of ls to better handle non-alphanumeric…
da043d5 
… filenames.
@shivaraj-bh
shell-check: read without -r will mangle backslashes
5cf1337
@shivaraj-bh
shell-check: Double quote to prevent globbing and word splitting.
f2a68d8 
+ Also don't unset PGHOST, the script fails with unbound variable error
+ Remove the unnecessary comment about PGHOST and PGPORT
@shivaraj-bh
use runtimeInputs of writeShellApplication
6c8b1bb
@shivaraj-bh
fmt
23d28b8
@shivaraj-bh
use apply attribute of mkOption to create postgresPkg with extensions
6940b2c
@shivaraj-bh
mv postgres.nix and postgres_test.nix to postgres folder
f55e096
@shivaraj-bh
Merge branch 'main' into update-postgres-to-psql
a6f06d3
@shivaraj-bh
rename postgres/test.nix as the name of the test is detected from the…
fa0de29 
… file name
@shivaraj-bh
Copy link
Member

@rsrohitsingh682 will merge this today if there is nothing more to add

@shivaraj-bh shivaraj-bh merged commit 22e121b into juspay:main Dec 24, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@srid srid srid left review comments

@shivaraj-bh shivaraj-bh shivaraj-bh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Postgres: use psql instead of postgres CLI to load the initialScript
3 participants
@rsrohitsingh682 @shivaraj-bh @srid