feat: add more config examples to CLI #3481

taimoorzaeem · 2024-05-03T19:03:40Z

Closes #3248.

docs/references/cli.rst

src/PostgREST/CLI.hs

wolfgangwalther · 2024-05-04T10:14:31Z

src/PostgREST/CLI.hs

+      |
+      |## The standard connection URI format, documented at
+      |## https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING
+      |ALTER ROLE authenticator SET pgrst.db_uri = 'postgresql://';


This is not a setting that is possible to set via database configuration. I suggest you go check the code which of those settings are actually possible to set via database config - because there's more than just this that don't make sense here at all.

src/PostgREST/CLI.hs

wolfgangwalther · 2024-05-04T10:19:15Z

CHANGELOG.md

+ - #3248, Add more config examples to CLI - @taimoorzaeem
+   + Now accepts ``--example-file``, ``--example-db``, and ``--example-env``


You also have two changes here, one to rename the dump schema cache command and the other to remove --example and -e. Those should probably go into the changed section as well.

wolfgangwalther · 2024-05-09T17:41:03Z

src/PostgREST/CLI.hs

+  [str|-- Enables the aggregate functions in postgresql
+      |-- ALTER ROLE authenticator SET pgrst.db_aggregates_enabled = 'false';


Seems like you added this - but only in the db settings. I guess we have quite a few settings which are not in those example config files by now, because that's always forgotten when adding a new config.

It would probably be good to go through all of them and add them to all three (two for those with are not db-configurable) examples.

wolfgangwalther · 2024-05-09T17:41:08Z

src/PostgREST/CLI.hs

+      |
+      |-- Unix socket location
+      |-- if specified it takes precedence over server-port
+      |-- ALTER ROLE authenticator SET pgrst.server_unix_socket = '/tmp/pgrst.sock';


Suggested change

|

|-- Unix socket location

|-- if specified it takes precedence over server-port

|-- ALTER ROLE authenticator SET pgrst.server_unix_socket = '/tmp/pgrst.sock';

This is not in the list of dbSettingsNames.

taimoorzaeem · 2024-05-11T08:14:12Z

@wolfgangwalther Hmm, I have noticed an inconsistency between docs and dbSettingsNames. The dbSettingsNames include db_tx_end but the docs configuration page says that db_tx_end is n/a for in-db config. More the dbSettingsNames include raw_media_types but there is no detail available around this config in the documentation.

wolfgangwalther · 2024-05-12T09:55:49Z

The dbSettingsNames include db_tx_end but the docs configuration page says that db_tx_end is n/a for in-db config.

A good catch. I added it in aef29d4.

More the dbSettingsNames include raw_media_types but there is no detail available around this config in the documentation.

We removed raw-media-types in v12.0.0 and replaced it with aggregation / media handlers. Removed the left-over in c6f152f.

wolfgangwalther · 2024-05-12T09:56:42Z

Please rebase, solving conflicts with CHANGELOG.md.

laurenceisla · 2024-05-20T18:21:04Z

CHANGELOG.md

+   + Replace ``-e`` and ``--example`` with ``--example-file``
+   + Rename option ``--dump-schema`` to ``--dump-schema-cache``


Since they're kinda breaking changes, these should go under a separate subtitle (after the ### Fixed entries like in the 10.1.0 changelog), something like:

### Changed - #3248, Modified some CLI options - @taimoorzaeem + Replace ``-e`` and ``--example`` with ``--example-file`` + Rename option ``--dump-schema`` to ``--dump-schema-cache``

laurenceisla

These should be commented since they do not have a default value, and should also show a sample value (similar to how db-anon-role works). Only checked for the changes in the --example-file, maybe others are missing (verify in the docs: if they have "Default: n/a" then they should be commented).

laurenceisla · 2024-05-20T18:47:20Z

src/PostgREST/CLI.hs

@@ -171,6 +187,9 @@ exampleConfigFile =
      |## When disabled, statements will be parametrized but won't be prepared.
      |db-prepared-statements = true
      |
+      |## Function to override OpenAPI response
+      |db-root-spec = ""


Suggested change

|db-root-spec = ""

|# db-root-spec = "root"

laurenceisla · 2024-05-20T18:47:27Z

src/PostgREST/CLI.hs

@@ -219,6 +241,9 @@ exampleConfigFile =
      |server-host = "!4"
      |server-port = 3000
      |
+      |## Trace HTTP request header
+      |server-trace-header = ""


Suggested change

|server-trace-header = ""

|# server-trace-header = "X-Request-Id"

laurenceisla · 2024-05-20T18:47:31Z

src/PostgREST/CLI.hs

+      |ALTER ROLE authenticator SET pgrst.db_prepared_statements = 'true';
+      |
+      |-- Function to override the OpenAPI response
+      |ALTER ROLE authenticator SET pgrst.db_root_spec = '';


Suggested change

|ALTER ROLE authenticator SET pgrst.db_root_spec = '';

|-- ALTER ROLE authenticator SET pgrst.db_root_spec = 'root';

laurenceisla · 2024-05-20T18:47:34Z

src/PostgREST/CLI.hs

+      |-- ALTER ROLE authenticator SET pgrst.server_cors_allowed_origins = '';
+      |
+      |-- Trace HTTP request header
+      |-- ALTER ROLE authenticator SET pgrst.server_trace_header = '';


Suggested change

|-- ALTER ROLE authenticator SET pgrst.server_trace_header = '';

|-- ALTER ROLE authenticator SET pgrst.server_trace_header = 'X-Request-Id';

laurenceisla · 2024-05-20T18:47:44Z

src/PostgREST/CLI.hs

+      |PGRST_DB_PREPARED_STATEMENTS=true
+      |
+      |## Function to override the OpenAPI response
+      |PGRST_DB_ROOT_SPEC=''


Suggested change

|PGRST_DB_ROOT_SPEC=''

|# PGRST_DB_ROOT_SPEC='root'

laurenceisla · 2024-05-20T18:48:07Z

src/PostgREST/CLI.hs

+      |PGRST_SERVER_PORT=3000
+      |
+      |## Trace HTTP request header
+      |PGRST_SERVER_TRACE_HEADER=''


Suggested change

|PGRST_SERVER_TRACE_HEADER=''

|# PGRST_SERVER_TRACE_HEADER='X-Request-Id'

wolfgangwalther requested changes May 4, 2024

View reviewed changes

taimoorzaeem force-pushed the feat/more-cli-examples branch from 2263bc0 to e503947 Compare May 5, 2024 19:11

wolfgangwalther reviewed May 9, 2024

View reviewed changes

taimoorzaeem added 2 commits May 13, 2024 00:42

feat: add more config examples to CLI

2529098

chore: rename option --dump-schema to --dump-schema-cache

cce425e

taimoorzaeem force-pushed the feat/more-cli-examples branch from e503947 to cce425e Compare May 12, 2024 19:59

Merge branch 'main' into feat/more-cli-examples

4c16e11

laurenceisla reviewed May 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add more config examples to CLI #3481

feat: add more config examples to CLI #3481

taimoorzaeem commented May 3, 2024 •

edited

wolfgangwalther May 4, 2024

wolfgangwalther May 4, 2024

wolfgangwalther May 9, 2024

wolfgangwalther May 9, 2024

taimoorzaeem commented May 11, 2024

wolfgangwalther commented May 12, 2024

wolfgangwalther commented May 12, 2024

laurenceisla May 20, 2024 •

edited

laurenceisla left a comment

laurenceisla May 20, 2024

laurenceisla May 20, 2024

laurenceisla May 20, 2024

laurenceisla May 20, 2024

laurenceisla May 20, 2024

laurenceisla May 20, 2024

		- #3248, Add more config examples to CLI - @taimoorzaeem
		+ Now accepts ``--example-file``, ``--example-db``, and ``--example-env``

		[str\|-- Enables the aggregate functions in postgresql
		\|-- ALTER ROLE authenticator SET pgrst.db_aggregates_enabled = 'false';

		+ Replace ``-e`` and ``--example`` with ``--example-file``
		+ Rename option ``--dump-schema`` to ``--dump-schema-cache``

	\|server-trace-header = ""
	\|# server-trace-header = "X-Request-Id"

	\|ALTER ROLE authenticator SET pgrst.db_root_spec = '';
	\|-- ALTER ROLE authenticator SET pgrst.db_root_spec = 'root';

	\|-- ALTER ROLE authenticator SET pgrst.server_trace_header = '';
	\|-- ALTER ROLE authenticator SET pgrst.server_trace_header = 'X-Request-Id';

	\|PGRST_SERVER_TRACE_HEADER=''
	\|# PGRST_SERVER_TRACE_HEADER='X-Request-Id'

feat: add more config examples to CLI #3481

Are you sure you want to change the base?

feat: add more config examples to CLI #3481

Conversation

taimoorzaeem commented May 3, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taimoorzaeem commented May 11, 2024

wolfgangwalther commented May 12, 2024

wolfgangwalther commented May 12, 2024

laurenceisla May 20, 2024 • edited

Choose a reason for hiding this comment

laurenceisla left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taimoorzaeem commented May 3, 2024 •

edited

laurenceisla May 20, 2024 •

edited