feat: support env variables to initialize db and user name #12545

lmatz · 2023-09-26T11:28:20Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

#11190 two out of three

since we have no default password being set, need someone help with the POSTGRES_PASSWORD one

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

Support using environment variables POSTGRES_USER and POSTGRES_DB to initialize both configs.

lmatz · 2023-09-26T11:28:35Z

codecov · 2023-09-26T13:21:18Z

Codecov Report

Merging #12545 (56574be) into main (09a1dcb) will increase coverage by 0.03%.
The diff coverage is 74.35%.

@@            Coverage Diff             @@
##             main   #12545      +/-   ##
==========================================
+ Coverage   69.40%   69.44%   +0.03%     
==========================================
  Files        1440     1440              
  Lines      238757   238772      +15     
==========================================
+ Hits       165713   165809      +96     
+ Misses      73044    72963      -81

Flag	Coverage Δ
rust	`69.44% <74.35%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/frontend/src/handler/alter_relation_rename.rs	`33.89% <100.00%> (ø)`
src/frontend/src/handler/alter_source_column.rs	`85.38% <100.00%> (ø)`
src/frontend/src/handler/alter_table_column.rs	`69.29% <100.00%> (ø)`
src/frontend/src/handler/create_mv.rs	`93.72% <100.00%> (ø)`
src/frontend/src/handler/create_schema.rs	`80.35% <100.00%> (ø)`
src/frontend/src/handler/create_sink.rs	`96.06% <100.00%> (+0.09%)`	⬆️
src/frontend/src/handler/create_source.rs	`48.56% <100.00%> (ø)`
src/frontend/src/handler/create_table.rs	`84.15% <100.00%> (ø)`
src/frontend/src/handler/drop_index.rs	`63.49% <100.00%> (ø)`
src/frontend/src/handler/drop_mv.rs	`76.27% <100.00%> (ø)`
... and 8 more

... and 10 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

BugenZhao · 2023-09-26T14:00:46Z

src/common/src/catalog/mod.rs

-pub const DEFAULT_DATABASE_NAME: &str = "dev";
+use std::env;
+
+pub static DEFAULT_DATABASE_NAME: LazyLock<String> =


I'm wondering whether we should make them system parameters and allow users to override them with CLI args on the first run. Or it could be confusing. 🤔

+1. BTW maybe it doesn't need to be a system param and just a CLI arg or sth, as it's only usage is init.

No confusion since the user must have looked up for the variable and know exactly what he wants to do.

The common usage is to specify in the docker file before starting the database.

IIUC, POSTGRES_DB is a feature of postgres docker image, instead of postgres kernel, which has other configuration mechanism for initialization.

So, I think this solution of course works, but it's hacky.

+1. We need to make sure it only takes effect on the first run. After the database being initialized, the default db and username should be persisted.

I believe this is also the behavior of the POSTGRES_DB.

BugenZhao · 2023-09-26T14:01:36Z

src/cmd_all/src/playground.rs

+        format!(
+            "psql -h localhost -p 4566 -d {} -U {}",
+            *DEFAULT_DATABASE_NAME, *DEFAULT_SUPER_USER
+        )


This actually reminds me of the PGxxx environment variables..

https://www.postgresql.org/docs/current/libpq-envars.html

lmatz · 2023-09-26T14:11:15Z

how about RW_?

No confusion since the user must have looked up for the variable and know exactly what he wants to do.

Was requested by three users.

The common usage is to specify in the docker file before starting the database.

fuyufjh · 2023-12-06T09:25:40Z

Ping. Shall we push this forward? I think it's blocking on #12545 (comment) and no updates since then. cc. @lmatz

lmatz · 2023-12-13T07:04:21Z

Sure, let me work on it in the near future,

I believe I misunderstood the previous comment above,

I suppose the key point is to persist and use the persistent default users and database names to do all kinds of comparisons and checks later, right

using environment variable to set the default users and database names is still acceptable❓

xxchan · 2023-12-13T07:21:25Z

FYI, this is how POSTGRES_DB works... Seems to just run some sql on startup.

https://github.com/docker-library/postgres/blob/def08559e5eebce85e6a9e8b21ff87546c4e7047/docker-entrypoint.sh#L186-L215

fuyufjh · 2023-12-15T04:00:45Z

using environment variable to set the default users and database names is still acceptable❓

Yeah, but only the first boot for initialization, as demonstrated in @xxchan's comment.

BugenZhao · 2024-04-11T05:19:07Z

Recap my idea:

Read configuration once on the first bootstrap of meta node.
Frontend nodes are not aware of this configuration. They should receive this information from the meta node during registration. Also, I don't come up with many cases that frontends have to what "default" is. IMO frontends always work under the database explicitly specified by users in their clients.

Where to read the configuration will be another orthogonal topic: either through env var to be compatible with postgres server, or through some (immutable) system parameters from the configuration file.

cc @yezizp2012

github-actions · 2024-06-12T09:00:30Z

This PR has been open for 60 days with no activity. Could you please update the status? Feel free to ping a reviewer if you are waiting for review.

lmatz requested a review from a team as a code owner September 26, 2023 11:28

github-actions bot added the type/feature label Sep 26, 2023

lmatz added the user-facing-changes Contains changes that are visible to users label Sep 26, 2023

lmatz requested a review from fuyufjh September 26, 2023 11:33

lmatz added 2 commits September 26, 2023 20:14

feat: support env variables to initialize db and user name

d52bbf7

Fix tests

56574be

lmatz force-pushed the lz/env_default branch from 38d4993 to 56574be Compare September 26, 2023 12:14

BugenZhao reviewed Sep 26, 2023

View reviewed changes

github-actions bot added the no-pr-activity label Jun 12, 2024

fuyufjh closed this Jun 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support env variables to initialize db and user name #12545

feat: support env variables to initialize db and user name #12545

lmatz commented Sep 26, 2023 •

edited

Loading

lmatz commented Sep 26, 2023

codecov bot commented Sep 26, 2023 •

edited

Loading

BugenZhao Sep 26, 2023

xxchan Sep 26, 2023

fuyufjh Nov 10, 2023

BugenZhao Sep 26, 2023

lmatz commented Sep 26, 2023 •

edited

Loading

fuyufjh commented Dec 6, 2023

lmatz commented Dec 13, 2023

xxchan commented Dec 13, 2023

fuyufjh commented Dec 15, 2023

BugenZhao commented Apr 11, 2024 •

edited

Loading

github-actions bot commented Jun 12, 2024

feat: support env variables to initialize db and user name #12545

feat: support env variables to initialize db and user name #12545

Conversation

lmatz commented Sep 26, 2023 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Release note

lmatz commented Sep 26, 2023

codecov bot commented Sep 26, 2023 • edited Loading

Codecov Report

BugenZhao Sep 26, 2023

Choose a reason for hiding this comment

xxchan Sep 26, 2023

Choose a reason for hiding this comment

fuyufjh Nov 10, 2023

Choose a reason for hiding this comment

BugenZhao Sep 26, 2023

Choose a reason for hiding this comment

lmatz commented Sep 26, 2023 • edited Loading

fuyufjh commented Dec 6, 2023

lmatz commented Dec 13, 2023

xxchan commented Dec 13, 2023

fuyufjh commented Dec 15, 2023

BugenZhao commented Apr 11, 2024 • edited Loading

github-actions bot commented Jun 12, 2024

lmatz commented Sep 26, 2023 •

edited

Loading

codecov bot commented Sep 26, 2023 •

edited

Loading

lmatz commented Sep 26, 2023 •

edited

Loading

BugenZhao commented Apr 11, 2024 •

edited

Loading