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

[16.0] Fix multi databases with list_db = false #589

Merged
merged 2 commits into from
Nov 23, 2023

Conversation

guewen
Copy link
Member

@guewen guewen commented Nov 22, 2023

Port of #379 , #571, #580

eLBati and others added 2 commits November 22, 2023 17:04
…tance with list_db = False

The scenario is a multi database environment where you can't set db_name as the databases change frequently and you use dbfilter to allow users to access their DB according to domain
@guewen guewen changed the title [14.0] Fix multi databases with list_db = false [16.0] Fix multi databases with list_db = false Nov 22, 2023
@pcastelovigo
Copy link
Contributor

thank you so much

@guewen
Copy link
Member Author

guewen commented Nov 23, 2023

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-589-by-guewen-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 4a99cb8 into OCA:16.0 Nov 23, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at f86ebbb. Thanks a lot for contributing to OCA. ❤️

Comment on lines +401 to +406
"""
>>> runner = QueueJobRunner()
>>> config["db_name"] = None
>>> runner.get_db_names()
['odoo']
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

What about add list_db = False to test odoo.service.db.list_dbs(True) against odoo.service.db.exp_list(True)?

>>> config["list_db"] = False

Copy link
Contributor

@pcastelovigo pcastelovigo Nov 23, 2023

Choose a reason for hiding this comment

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

i'm not sure about the implications, as I must confess I never checked what list_dbs or exp_list does

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, i finally understood. Yes, thats a great idea, and a much better test approach
This only checks list_dbs gives something like untested exp_lists, your test checks if it works without list_dbs

Copy link
Contributor

Choose a reason for hiding this comment

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

:) Please see #591

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

Successfully merging this pull request may close these issues.

6 participants