[19.0] [MIG] queue_job: migrate + tests

[tishmen]

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.

[sbidoul]

/ocabot migration queue_job

[sbidoul]

/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

[tishmen]

@hoangtrann quick update:

• We’re down to just 3 open comments on this PR; the rest are addressed and pushed.
• When you have a moment, a fresh check would be super helpful.

I’d also really appreciate it if you could take a look at PR #842—specifically the tests:

You can run the suite locally with (use any throwaway DB name in place of <DB_NAME>):

./odoo/odoo-bin -c odoo.conf -d <DB_NAME>
--init queue_job,queue_job_batch,queue_job_cron,queue_job_cron_jobrunner,queue_job_subscribe,test_queue_job,test_queue_job_batch
--test-tags="/queue_job,/queue_job_batch,/queue_job_cron,/queue_job_cron_jobrunner,/queue_job_subscribe,/test_queue_job,/test_queue_job_batch"
--stop-after-init

Thanks again for the thorough and detailed review—really grateful for your contributions and feedback 🙏

[hoangtrann]

there are still some comments stating odoo 19 scattered across the changes which I thing we should removed.

for anyone comes after these are removed, I think it's good to just summarized the major changes to the description of the pr which can also works as a good resource for reference in other mig pr

[tishmen]

there are still some comments stating odoo 19 scattered across the changes which I thing we should removed.

for anyone comes after these are removed, I think it's good to just summarized the major changes to the description of the pr which can also works as a good resource for reference in other mig pr

Resolved in: d27f83d

[hoangtrann]

LGTM 👍

[richard-willdooit]

From my checks and tests, this is all ready to go. Can it be merged?

@sbidoul

[PieterPaulussen]

@tishmen have you tested this version with the preforkserver configuration (using --workers=4)? Jobs are enqueued, but never started.
The threaded server setup does work.

Additionally, there is an error due to the upstream config refactoring in this method:
https://github.com/tishmen/queue/blob/pr/queue_job_base/queue_job/jobrunner/runner.py#L474-L479. On the line db_names = config["db_name"].split(",") specifically: config["db_name"] is no longer a comma separated string, but already a list so the split can be omitted.

[PieterPaulussen]

@tishmen An additional refactoring is necessary here because read_group is no longer supported by the core. You'll have to refactor this into the use of the new _read_group method.

[nilshamerlinck]

@PieterPaulussen fyi, workers mode working fine here, is the runner starting properly on your end?

$ egrep "(runner|runner\.channels): .*(starting|initializing|.+runner ready|database connections ready|Configured channel)" odoo.log

[PieterPaulussen]

@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.

[PieterPaulussen]

@PieterPaulussen fyi, workers mode working fine here, is the runner starting properly on your end?

$ egrep "(runner|runner\.channels): .*(starting|initializing|.+runner ready|database connections ready|Configured channel)" odoo.log

@nilshamerlinck Curious, could you share your worker and channel configuration please? In my test, the jobs got enqueued, but were never actually processed.

[maciej-wichowski]

@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)

[tishmen]

@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]

@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]

@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]

@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]

@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]

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.

[PieterPaulussen]

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

[OCA-git-bot]

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]

@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]

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]

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]

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]

@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.

[tishmen]

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

[em230418]

Здраво, @tishmen!

@sbidoul I will take care of all of the code review comments this weekend.

Hope you had nice weekend :D

[tishmen]

Sorry for the delay — I’ve pushed updates addressing the review feedback (style nits, test cleanups, and the _read_group perf tweak). Pre-commit is green again. Please take another look when you have a moment.

[etobella]

@sbidoul can we help somehow on this?

Just a question, there is need for so many migration commits or should it be squashed in a single one?

[sbidoul]

I had these last comments pending.

[sbidoul]

About the commits, this helped during review but I suppose it's best to squash them now. Then we can merge.

[tishmen]

Hi all! I’ve now squashed PR #840 down to a single commit as requested, and kept the latest review fixes in place. Pre-commit and an Odoo --stop-after-init install check are green on my side. Could you please re-review when you have a moment? @sbidoul @etobella