[19.0] [MIG] queue_job: migrate + tests #840

tishmen · 2025-10-01T16:03:37Z

Scope

queue_job, test_queue_job only

Summary

Migrate to Odoo 19 using the working feature branch code.
Tests create their own users (no demo data), context handling aligned with 19.0.
Asset paths are relative under web.assets_backend.

Pre-commit

Verified locally; no additional changes applied here to keep parity with the feature branch.

Tests

Command:
./odoo/odoo-bin -c odoo.conf -d odoo_queue_pr_queue_job_base_001 --init queue_job,test_queue_job --test-tags="/queue_job,/test_queue_job" --stop-after-init
Result: all tests green locally.

Scope: queue_job, test_queue_job only

…tallable)

…config.yaml excluded-addons update\n- Prettier reformat XML/JS/SCSS and pyproject changes\n- Fix ruff UP031 in controllers and job repr/exception

…._ in controller\n- Use self.env._ + lazy %% formatting in models\n- Paginate channel search in autovacuum\n- Avoid raise in unlink; skip root channel\n- Fix tests: avoid search([]) and correct pylint disable id

sbidoul · 2025-10-01T21:24:17Z

/ocabot migration queue_job

sbidoul · 2025-10-01T21:24:45Z

/ocabot migration test_queue_job

hoangtrann

appreciate all the work you done here! please review the comments since I notice many weird comments and many changes should be in another PR instead of a migration one

queue_job/controllers/main.py

queue_job/fields.py

queue_job/job.py

queue_job/views/queue_job_views.xml

test_queue_job/tests/test_job.py

test_queue_job/tests/test_json_field.py

… error messages; keep logging/i18n/SQL intact

…(guard remains in ondelete)

…where appropriate

…ncer (#no-search-all)

…locally (#no-search-all)

…hannel model

Reyes4711-S73

LGTM

omarsyscoon

LGTM

- Remove extra space before "model" in value_json for test_decoder_recordset_list_without_user - Align expected string with encoder output to avoid whitespace-only assertion failures - Scope: tests only; no functional changes Co-authored-by: Pieter Paulussen <[email protected]>

tishmen · 2025-11-10T22:11:19Z

@tishmen Please take a look at this commit. I would create a PR for your branch, but for some reason I couldn't do it.

It's a fix related to a change in db_name parsing from config (src)

Handled in update.

tishmen · 2025-11-10T22:12:44Z

@tishmen jobs_groups = self.env["queue.job"].read_group(

I get: DeprecationWarning: Since 19.0, read_group is deprecated. Please use _read_group might be good to get rid of this?

Handled in update.

tishmen · 2025-11-10T22:13:18Z

@tishmen i have this when i initialise new db,

DR-ODOO  | 2025-11-04 12:13:16,520 1 DEBUG ? odoo.addons.queue_job.jobrunner.runner: initializing database connections 
DR-ODOO  | 2025-11-04 12:13:16,520 1 ERROR ? odoo.addons.queue_job.jobrunner.runner: exception: sleeping 5s and retrying 
DR-ODOO  | Traceback (most recent call last):
DR-ODOO  |   File "/mnt/odoo/external_addons/queue_job/jobrunner/runner.py", line 597, in run
DR-ODOO  |     self.initialize_databases()
DR-ODOO  |   File "/mnt/odoo/external_addons/queue_job/jobrunner/runner.py", line 492, in initialize_databases
DR-ODOO  |     for db_name in sorted(self.get_db_names()):
DR-ODOO  |   File "/mnt/odoo/external_addons/queue_job/jobrunner/runner.py", line 476, in get_db_names
DR-ODOO  |     db_names = config["db_name"].split(",")
DR-ODOO  | AttributeError: 'list' object has no attribute 'split'
DR-ODOO  | 2025-11-04 12:13:17,230 1 INFO 19_controller3 odoo.modules.loading: loading base/data/report_paperformat_data.xml

Handled in update.

tishmen · 2025-11-10T22:15:24Z

@tishmen i have this when i initialise new db,

DR-ODOO  | 2025-11-04 12:13:16,520 1 DEBUG ? odoo.addons.queue_job.jobrunner.runner: initializing database connections 
DR-ODOO  | 2025-11-04 12:13:16,520 1 ERROR ? odoo.addons.queue_job.jobrunner.runner: exception: sleeping 5s and retrying 
DR-ODOO  | Traceback (most recent call last):
DR-ODOO  |   File "/mnt/odoo/external_addons/queue_job/jobrunner/runner.py", line 597, in run
DR-ODOO  |     self.initialize_databases()
DR-ODOO  |   File "/mnt/odoo/external_addons/queue_job/jobrunner/runner.py", line 492, in initialize_databases
DR-ODOO  |     for db_name in sorted(self.get_db_names()):
DR-ODOO  |   File "/mnt/odoo/external_addons/queue_job/jobrunner/runner.py", line 476, in get_db_names
DR-ODOO  |     db_names = config["db_name"].split(",")
DR-ODOO  | AttributeError: 'list' object has no attribute 'split'
DR-ODOO  | 2025-11-04 12:13:17,230 1 INFO 19_controller3 odoo.modules.loading: loading base/data/report_paperformat_data.xml

I have this issue as well, are you on Odoo.sh?

no i was testing locally,

temporary i solved by the following jobrunner/runner.py L#474


    def get_db_names(self):
        if config["db_name"]:
            if isinstance(config["db_name"], list):
                return config["db_name"]
            return config["db_name"].split(",")
        return odoo.service.db.list_dbs(True)

I am going to use a similar solution to yours to handle this and not the PR that was suggested above. Thanks.

tishmen · 2025-11-10T22:28:14Z

@tishmen Thanks for your effort on this. On the whole, this PR does the trick, but there are a lot of additional changes that are not essential to the module migration. In my opinion, you will get faster approvals if you slim down the changes to the bare minimum given the complexity of this module.

If you want to keep the translations in this PR, can we go one step further and use positional arguments for all translated string variables? This results in more flexibility for other languages during translating.

On the refactoring of the tests to remove the reliance on the demo_user, I would beg to differ. Unittests should rely on the presence of the demo data, just as the core does.

Finally, I would like to stress the following issue that I encountered during a local test with your code: #840 (comment) The jobrunner does not seem to enqueue AND process the queue jobs when running is preforked server configuration. This issue should be addressed first.

Thanks for the review. I’ve updated the PR to use positional arguments for all translated strings to improve i18n flexibility. Regarding the tests: in Odoo 19, demo data isn’t loaded by default during test runs, which caused failures when relying on the demo user, so the tests create a user explicitly to stay deterministic

aisopuro · 2025-11-12T15:13:12Z

For extra context, Odoo indeed changed the default for loading demo data in odoo/odoo@eeac3d4.

They even explicitly state in the commit message:

In testing we should not rely on demo data, but instead data constructed during the test.

I at least would interpret this to mean that part of migrating code to 19 is to rewrite/refactor tests so they no longer depend on demo data.

mostafabarmshory

@tishmen
@sbidoul
@PieterPaulussen

Please review and approve this PR. We need this branch, and progress is blocked by non-critical issues.

mostafabarmshory · 2025-11-17T11:25:51Z

test_queue_job/tests/test_job.py

+                "company_ids": [(6, 0, [main_company.id])],
+                "group_ids": [(6, 0, [group_user.id])],


@tishmen
@ sbidoul

Please review and approve this PR. We need this branch, and progress is blocked by non-critical issues.

PieterPaulussen

Most of my feedback seems to be either ignored or resolved.

OCA-git-bot · 2025-11-17T11:50:23Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

Robrechtc · 2025-11-19T12:41:18Z

@pedrobaeza Sorry for tagging you here again but you are the only one that I know has merge rights. Would you mind merging this? Thanks in advance.

pedrobaeza · 2025-11-19T12:45:13Z

Hi, I'm afraid I don't know too much this module, so one PSC like @guewen, @simahawk or @sbidoul should check that it's correct and then merge.

Robrechtc · 2025-11-19T12:48:04Z

Hi, I'm afraid I don't know too much this module, so one PSC like @guewen, @simahawk or @sbidoul should check that it's correct and then merge.

Thanks for tagging them.

sbidoul · 2025-11-19T14:48:41Z

I plan to read the latest comments and re-review soon(ish). Maybe next week.

It the meantime, nothing prevents using it, and reporting further issues if any.

richard-willdooit · 2025-11-20T08:17:01Z

@tishmen Well done - all I can really say is, that I'm glad my PR was superseded - 😜

Overall, I really think we should be getting these PRs for migrations through as efficiently as possible. I know everyone has their opinions, and where code has been touched or altered, it is great to aim for perfection, but if the changes are not wrong, if they are not regressive, then there should be no obligation to "improve" during a migration. Nice if it does, but it should not be obligatory.

After watching this unfold, I will be VERY uninclined to offer a module migration, as my objective will be to get the module across as is, and correct, not suddenly inheriting everyone else's wishlists...

Of course, this does not mean dismissing anyone's reviews or input without due and proper consideration, but the end game of a migration PR seems different for different people....

sbidoul

Thanks, this looks great now.

A few last minor things, and one more important one for performance of graph handling in the read_group rewrite.

sbidoul · 2025-11-20T13:08:18Z

queue_job/jobrunner/runner.py

+        if db_name_opt:
+            if isinstance(db_name_opt, (list, tuple, set)):
+                return list(db_name_opt)
+            return [n for n in str(db_name_opt).split(",") if n]


Does any one know in which case config["db_name"] is not a list? As I read odoo/tools/config.py it seems it's always a list, since it is parsed with _check_comma.

sbidoul · 2025-11-20T13:08:59Z

queue_job/models/base.py

+    # Note: no local _patch_method helper; if needed, patch methods
+    # directly in _register_hook as shown above.


Suggested change

# Note: no local _patch_method helper; if needed, patch methods

# directly in _register_hook as shown above.

Nit: this comment is not really helpful outside the migration PR.

sbidoul · 2025-11-22T10:26:10Z

queue_job/models/queue_job.py

        index_1 = "queue_job_identity_key_state_partial_index"
        index_2 = "queue_job_channel_date_done_date_created_index"


nit: these two variables are not helpful, they could be inlined

sbidoul · 2025-11-22T11:03:06Z

queue_job/models/queue_job.py

-        ids_per_graph_uuid = {
-            group["graph_uuid"]: group["ids"] for group in jobs_groups
-        }
+        uuids = [uuid for uuid in self.mapped("graph_uuid") if uuid]


Suggested change

uuids = [uuid for uuid in self.mapped("graph_uuid") if uuid]

graph_uuids = [uuid for uuid in self.mapped("graph_uuid") if uuid]

To avoid confusion with job uuids.

sbidoul · 2025-11-22T11:05:02Z

queue_job/models/queue_job.py

+            rows = self.env["queue.job"]._read_group(
+                [("graph_uuid", "in", uuids)],
+                groupby=["graph_uuid"],
+                aggregates=["id:recordset"],
+            )
+            # rows -> list of tuples: (graph_uuid, recordset)
+            for graph_uuid, recs in rows:
+                ids_per_graph_uuid[graph_uuid] = recs.ids


Suggested change

rows = self.env["queue.job"]._read_group(

[("graph_uuid", "in", uuids)],

groupby=["graph_uuid"],

aggregates=["id:recordset"],

)

# rows -> list of tuples: (graph_uuid, recordset)

for graph_uuid, recs in rows:

ids_per_graph_uuid[graph_uuid] = recs.ids

ids_per_graph_uuid = dict(self.env["queue.job"]._read_group(

[("graph_uuid", "in", graph_uuids)],

groupby=["graph_uuid"],

aggregates=["id:array_agg"],

))

This is simpler and more efficient, as we don't need the recordsets.

sbidoul · 2025-11-22T11:08:29Z

queue_job/models/queue_job.py

+            rows = self.env["queue.job"]._read_group(
+                [("graph_uuid", "in", graph_uuids)],
+                ["graph_uuid"],
+                ["__count"],
+            )
+            count_per_graph_uuid = {graph_uuid: cnt for graph_uuid, cnt in rows}


nit, this is probably slightly more efficient than the dict comprehension

Suggested change

rows = self.env["queue.job"]._read_group(

[("graph_uuid", "in", graph_uuids)],

["graph_uuid"],

["__count"],

)

count_per_graph_uuid = {graph_uuid: cnt for graph_uuid, cnt in rows}

count_per_graph_uuid = dict(self.env["queue.job"]._read_group(

[("graph_uuid", "in", graph_uuids)],

groupby=["graph_uuid"],

aggregates=["__count"],

))

queue_job/tests/common.py

tishmen · 2025-11-26T17:03:12Z

@sbidoul I will take care of all of the code review comments this weekend. Thanks for your feedback.

Milan Topuzov added 11 commits October 1, 2025 18:01

[19.0] queue_job: migrate + tests

2a5ccb8

Scope: queue_job, test_queue_job only

[pre-commit] update excluded addons (queue_job/test_queue_job now ins…

ce67593

…tallable)

[pre-commit] apply hook updates + ruff fixes\n\n- Commit .pre-commit-…

b283cd8

…config.yaml excluded-addons update\n- Prettier reformat XML/JS/SCSS and pyproject changes\n- Fix ruff UP031 in controllers and job repr/exception

[lint] ruff fixes (E402, UP031, E501)

df50b83

[pre-commit] ruff-format applied

b7129d7

[pre-commit] ruff and format auto-fixes

a7d4364

[lint] tests: use odoo.tools.groupby instead of itertools.groupby

4da9fae

[pre-commit] reordered imports (ruff)

f494bcc

[lint] lazy translations + test groupby fix

3b733f1

[fix] channel: enforce root deletion guard in ondelete hook

ee67632

tishmen changed the title ~~[19.0] queue_job: migrate + tests~~ [19.0] [MIG] queue_job: migrate + tests Oct 1, 2025

OCA-git-bot added this to the 19.0 milestone Oct 1, 2025

OCA-git-bot mentioned this pull request Oct 1, 2025

Migration to version 19.0 #831

Open

8 tasks

hoangtrann suggested changes Oct 3, 2025

View reviewed changes

Milan Topuzov added 7 commits October 4, 2025 23:35

style(queue): convert safe string formatting to f-strings and extract…

d827d58

… error messages; keep logging/i18n/SQL intact

tests(doctest): reset optionflags to 0; remove no-op unlink override …

9aeebee

…(guard remains in ondelete)

style(queue): use PEP 604 unions in isinstance checks (Python 3.10+) …

9ac6b25

…where appropriate

style(queue): extract i18n raise messages into variables for consistency

99cc44f

autovacuum: restore original channel iteration form; keep linter sile…

211e467

…ncer (#no-search-all)

tests(common): restore search([]) for readability and silence linter …

620413a

…locally (#no-search-all)

chore(queue): remove unnecessary migration comments in security and c…

82d62a1

…hannel model

Reyes4711-S73 approved these changes Nov 6, 2025

View reviewed changes

omarsyscoon approved these changes Nov 10, 2025

View reviewed changes

Milan Topuzov and others added 5 commits November 10, 2025 23:05

jobrunner: support list-valued db_name

96a9ada

models(queue_job): compute graph count with _read_group

afc9c76

tests: use Command API for m2m

c30bf4b

pre-commit: apply autofixes (formatting, lint passes)

123900c

tishmen force-pushed the pr/queue_job_base branch from ce3a0d3 to 123900c Compare November 10, 2025 22:05

jobrunner: simplify get_db_names (no split, no parsing)

d25ff12

jobrunner: restore list/CSV handling in get_db_names (no comments)

754e438

astirpe mentioned this pull request Nov 14, 2025

[19.0][MIG] account_credit_control_queue_job OCA/credit-control#521

Draft

3 tasks

mostafabarmshory approved these changes Nov 17, 2025

View reviewed changes

PieterPaulussen approved these changes Nov 17, 2025

View reviewed changes

OCA-git-bot added approved ready to merge labels Nov 17, 2025

guewen approved these changes Nov 21, 2025

View reviewed changes

sbidoul reviewed Nov 22, 2025

View reviewed changes

		"company_ids": [(6, 0, [main_company.id])],
		"group_ids": [(6, 0, [group_user.id])],

		# Note: no local _patch_method helper; if needed, patch methods
		# directly in _register_hook as shown above.

		index_1 = "queue_job_identity_key_state_partial_index"
		index_2 = "queue_job_channel_date_done_date_created_index"

	uuids = [uuid for uuid in self.mapped("graph_uuid") if uuid]
	graph_uuids = [uuid for uuid in self.mapped("graph_uuid") if uuid]

Uh oh!

[19.0] [MIG] queue_job: migrate + tests #840

Are you sure you want to change the base?

[19.0] [MIG] queue_job: migrate + tests #840

Conversation

tishmen commented Oct 1, 2025

Uh oh!

sbidoul commented Oct 1, 2025

Uh oh!

sbidoul commented Oct 1, 2025

Uh oh!

hoangtrann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reyes4711-S73 left a comment

Choose a reason for hiding this comment

Uh oh!

omarsyscoon left a comment

Choose a reason for hiding this comment

Uh oh!

tishmen commented Nov 10, 2025

Uh oh!

tishmen commented Nov 10, 2025

Uh oh!

tishmen commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tishmen commented Nov 10, 2025

Uh oh!

tishmen commented Nov 10, 2025

Uh oh!

aisopuro commented Nov 12, 2025

Uh oh!

mostafabarmshory left a comment

Choose a reason for hiding this comment

Uh oh!

mostafabarmshory Nov 17, 2025

Choose a reason for hiding this comment

Uh oh!

PieterPaulussen left a comment

Choose a reason for hiding this comment

Uh oh!

OCA-git-bot commented Nov 17, 2025

Uh oh!

Robrechtc commented Nov 19, 2025

Uh oh!

pedrobaeza commented Nov 19, 2025

Uh oh!

Robrechtc commented Nov 19, 2025

Uh oh!

sbidoul commented Nov 19, 2025

Uh oh!

richard-willdooit commented Nov 20, 2025

Uh oh!

sbidoul left a comment

Choose a reason for hiding this comment

Uh oh!

sbidoul Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

sbidoul Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

sbidoul Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

sbidoul Nov 22, 2025

Choose a reason for hiding this comment

Uh oh!

sbidoul Nov 22, 2025

tishmen commented Nov 10, 2025 •

edited

Loading