queue icon indicating copy to clipboard operation
queue copied to clipboard

[19.0] [MIG] queue_job: migrate + tests

Open tishmen opened this issue 2 months ago • 28 comments

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.

tishmen avatar Oct 01 '25 16:10 tishmen

/ocabot migration queue_job

sbidoul avatar Oct 01 '25 21:10 sbidoul

/ocabot migration test_queue_job

sbidoul avatar Oct 01 '25 21:10 sbidoul

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

tishmen avatar Oct 04 '25 23:10 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: https://github.com/OCA/queue/pull/840/commits/d27f83d8c4b3a0a2a41c7cb2c71dc555db20bc0e

tishmen avatar Oct 11 '25 13:10 tishmen

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

@sbidoul

richard-willdooit avatar Oct 21 '25 06:10 richard-willdooit

@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 avatar Oct 27 '25 07:10 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.

PieterPaulussen avatar Oct 27 '25 10:10 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

image

nilshamerlinck avatar Oct 28 '25 08:10 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

image

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

PieterPaulussen avatar Oct 28 '25 23:10 PieterPaulussen

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

maciej-wichowski avatar Nov 01 '25 14:11 maciej-wichowski

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

CasVissers-360ERP avatar Nov 04 '25 05:11 CasVissers-360ERP

@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 

omarsyscoon avatar Nov 04 '25 12:11 omarsyscoon

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

CasVissers-360ERP avatar Nov 04 '25 12:11 CasVissers-360ERP

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

omarsyscoon avatar Nov 04 '25 12:11 omarsyscoon

@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 avatar Nov 10 '25 22:11 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 avatar Nov 10 '25 22:11 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 avatar Nov 10 '25 22:11 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 avatar Nov 10 '25 22:11 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

tishmen avatar Nov 10 '25 22:11 tishmen

For extra context, Odoo indeed changed the default for loading demo data in https://github.com/odoo/odoo/commit/eeac3d43a645bf7bb31b38c951d860163de7a458.

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.

aisopuro avatar Nov 12 '25 15:11 aisopuro

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). 🤖

OCA-git-bot avatar Nov 17 '25 11:11 OCA-git-bot

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

Robrechtc avatar Nov 19 '25 12:11 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.

pedrobaeza avatar Nov 19 '25 12:11 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.

Thanks for tagging them.

Robrechtc avatar Nov 19 '25 12:11 Robrechtc

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.

sbidoul avatar Nov 19 '25 14:11 sbidoul

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

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

richard-willdooit avatar Nov 20 '25 08:11 richard-willdooit

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

tishmen avatar Nov 26 '25 17:11 tishmen

Здраво, @tishmen!

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

Hope you had nice weekend :D

em230418 avatar Dec 12 '25 05:12 em230418