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

Support existing-but-not-initialized databases in init script #62

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

Conversation

yajo
Copy link
Contributor

@yajo yajo commented Nov 29, 2019

Since odoo/odoo@cb2862a (v12+), Odoo will not autoinitialize modules in a database that already exists.

However, in v8-, Odoo will not start if the database doesn't exist.

To make this difference easier to handle, with this change, click-odoo-initdb will treat non-existing databases the same as non-initialized ones (which is, for Odoo, the same).

@Tecnativa TT20713

@yajo yajo force-pushed the initdb-initialized-check branch from 08654d4 to 083d8c2 Compare November 29, 2019 10:09
Copy link
Member

@sbidoul sbidoul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you elaborate your practical use case?

click_odoo_contrib/initdb.py Outdated Show resolved Hide resolved
@yajo
Copy link
Contributor Author

yajo commented Nov 29, 2019

Yes, when booting a new instance, if the database exists but is not initialized, Odoo fails to start.

I expected click-odoo-initdb -n $PGDATABASE --unless-exists to initialize the DB, but it doesn't because it already exists.

IMHO, within Odoo's mindset a database "exists" if it is present in the server and initialized. I can however change some semantics if you prefer it, adding a --unless-initialized flag that does this specific behavior.

@sbidoul
Copy link
Member

sbidoul commented Nov 29, 2019

adding a --unless-initialized flag

I'd be more comfortable with that yes, with a test.

Thanks!

@yajo yajo force-pushed the initdb-initialized-check branch from 083d8c2 to 8eaf3d7 Compare December 2, 2019 09:06
@yajo
Copy link
Contributor Author

yajo commented Dec 2, 2019

Done.

@sbidoul
Copy link
Member

sbidoul commented Jan 5, 2020

This is still red.

@yajo
Copy link
Contributor Author

yajo commented Jan 7, 2020

AFAICS the problem is now the test, right? It's surprised that we can actually initialize a database that already existed, but since we didn't pass --unless-initialized, this is the new expected behavior. Is that it?

@sbidoul
Copy link
Member

sbidoul commented Jan 7, 2020

@yajo, the failing test looks ok to me: if we try initdb on any existing, even uninitialized database, it must fail by default as this is the safest thing to do.

@yajo
Copy link
Contributor Author

yajo commented Jan 8, 2020

I'm scratching a lot my head with these flags... 🤔 I think they don't make any sense.

Look, this matrix is what we'd got if we continue like this:

command exit code if db is absent action if db is absent exit code if db exists action if db exists exit code if db is initialized action if db is initialized
click-odoo-initdb -n newdb 0 Create + initialize 1 none 1 none
click-odoo-initdb -n newdb –unless-exists 0 Create + initialize 0 none 0 none
click-odoo-initdb -n newdb –unless-initialized 0 Create + initialize 0 initialize 0 none
click-odoo-initdb -n newdb –unless-initialized –but-tell-me-if-it-fails 0 Create + initialize 0 initialize 1 none

The last row is what is missing, and it is the case that I actually need. So, how do you want it implemented? Do I really have to add --but-tell-me-if-it-fails flag?

Under Odoo PoV, a DB that is not initialized is just as useless as if it didn't exist. So, can't we just make this script treat unexisting and uninitialized dbs the same? It'd be the simplest solution AFAICS...

@sbidoul
Copy link
Member

sbidoul commented Jan 13, 2020

Still scratching my head too ;) I'm still convinced we need a new option but I'm not sure yet how to name it.

@yajo
Copy link
Contributor Author

yajo commented Jan 13, 2020

can't we just make this script treat unexisting and uninitialized dbs the same?

The script is called initdb, not createdb. Would this be such a bad option? 🙄

@sbidoul sbidoul added the enhancement New feature or request label Mar 27, 2020
Since odoo/odoo@cb2862a (v12+), Odoo will not autoinitialize modules in a database that already exists.

However, in v8-, Odoo will not start if the database doesn't exist.

To make this difference easier to handle, with this change, `click-odoo-initdb` will treat non-existing databases the same as non-initialized ones (which is, for Odoo, the same).

@Tecnativa TT20713
@yajo yajo force-pushed the initdb-initialized-check branch from 8eaf3d7 to c5d44e9 Compare April 24, 2020 08:23
@pedrobaeza
Copy link

Any news on this?

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

Successfully merging this pull request may close these issues.

3 participants