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.

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

parameterize db_name #426

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

parameterize db_name #426

wants to merge 1 commit into from

Conversation

mohammed90
Copy link

@mohammed90 mohammed90 commented Nov 19, 2022

In case the database name is not postgres, the current image completely fails to run with the error:

Database connection failure: connection to server at "..." (ip-address), port ... failed: FATAL:  database "postgres" does not exist

This PR parameterizes the db name so the admin can spin up the image without having to hack-around this limitation.

Fixes #394

@mschinis
Copy link

mschinis commented Apr 4, 2023

can this please be merged?

@swissbuechi
Copy link

Thanks a lot. I hope it gets merged soon.

@amh-mw
Copy link

amh-mw commented Dec 20, 2023

Per https://www.postgresql.org/docs/current/app-initdb.html,

Creating a database cluster consists of creating the directories in which the cluster data will live, generating the shared catalog tables (tables that belong to the whole cluster rather than to any particular database), and creating the postgres, template1, and template0 databases. The postgres database is a default database meant for use by users, utilities and third party applications. template1 and template0 are meant as source databases to be copied by later CREATE DATABASE commands. template0 should never be modified, but you can add objects to template1, which by default will be copied into databases created later. See Section 23.3 for more details.

Hence, the startup sequence for this container checks for the default database to be available, verifying the readiness of the postgres container, rather than verifying the existence of a particular Odoo database -- of which there may be more than one.

I already ran this down in #482 and closed my own pull request because I determined this is a bad idea.

@mohammed90
Copy link
Author

Hence, the startup sequence for this container checks for the default database to be available, verifying the readiness of the postgres container, rather than verifying the existence of a particular Odoo database -- of which there may be more than one.

It can check for the database I command it to check, not the database the booting script decides to check. I'll share one set of credentials to the Odoo app, not 2, for the purpose of isolation and security. The current setup is a nightmare for multitenant hosting.

@amh-mw
Copy link

amh-mw commented Dec 20, 2023

I understand your reasoning, but in this pull request, the additional --db_name argument to wait-for-psql.py is never passed from entrypoint.sh. When I got that far in my own experimentation, I noticed that entrypoint.sh sends the same arguments to both wait-for-psql.py and odoo, which causes non-multi-tenant setups to run Odoo out of the postgres database by default... which is less than ideal.

@mohammed90
Copy link
Author

which causes non-multi-tenant setups to run Odoo out of the postgres database by default... which is less than ideal.

It's doing that already without the PR, which is... Less than ideal.

@lathama
Copy link

lathama commented Apr 2, 2024

@mohammed90 can you use https://docs.docker.com/build/building/multi-stage/ to accomplish your use case? If so can you close this PR?

@mohammed90
Copy link
Author

@mohammed90 can you use https://docs.docker.com/build/building/multi-stage/ to accomplish your use case? If so can you close this PR?

I don't understand how multi-stage build is supposed to help. Can you elaborate?

@lathama
Copy link

lathama commented Apr 2, 2024

I guess it does not need to be multi stage

Dockerfile

FROM docker.io/odoo:17.0 as odoodbname
COPY myversion_wait-for-psql.py /usr/local/bin/wait-for-psql.py

Then build

docker build -t odoo:myversion

Then you have your version.

Note: quickly typed out, spelling and typos are free of charge. :)

@mohammed90
Copy link
Author

I guess it does not need to be multi stage

Dockerfile

FROM docker.io/odoo:17.0 as odoodbname
COPY myversion_wait-for-psql.py /usr/local/bin/wait-for-psql.py

Then build

docker build -t odoo:myversion

Then you have your version.

Note: quickly typed out, spelling and typos are free of charge. :)

I fail to see how that is a proper solution. It's a hack at best. Appreciate the help, but I'm aiming to provide a solution that resolves root concern, not bandaid it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to specify the database name?
6 participants