-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Editing PostgreSQL image content for style and consistency #270
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
Conversation
|
Ping @mnagy for Tech Review. Also: Please provide default values for "PostgreSQL Settings" environment variables (see the table I've created). Trying to make this consistent with docs I'm doing for other images. |
openshift_images/postgresql.adoc
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could use the example without passing the password on a command line and instead enter it in the prompt? Since using the command line is generally considered insecure, we probably shouldn't give a bad example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a great point. We definitely don't want to recommend our users to do something that isn't secure.
|
@tpoitras Technically, all looks ok, thought I've made some comments. You can find the defaults in the postgresql repository's README.md. It's 100 connections and "32M" for shared buffers. |
|
Oh, and one more thing, that should maybe apply to all images. In order to build docker images, one needs to have docker working. Should we mention that? Possibly with the minimal required version I imagine, though I don't know off the top of my head which one that would be. |
|
Updated as per tech review from Martin. ping @mnagy -- please verify this looks good, specifically can you please make sure I got the syntax correct for the testing command, now that password is removed? I assumed that the colon should be removed as well. Also, I think the overview at the start of the topic already covers the need for Docker. |
|
@tpoitras I apologize, but I just realized that this might not work, because the psql command will prompt for the password and I'm not sure if redirecting the "SELECT 1;' statement won't get caught by the password prompt. Still, this is easily fixed, we just need to use -c or --command. or As per http://www.postgresql.org/docs/9.2/static/app-psql.html |
df99fa0 to
1a6593c
Compare
|
@mnagy -- thanks for that. Replaced with the 1st of your 2 suggested commands. Look good? |
|
LGTM |
|
this is ready to be merged now @CowboysFan @adellape |
|
Thanks, @tpoitras. LGTM as well, merging. |
Editing PostgreSQL image content for style and consistency
ensure the VS code extension section is worded correctly
Edited PostgreSQL image content from the following PRs:
#240
#247
As per Trello Card https://trello.com/c/GZ0BoR0X