[FIX] queue_job : check if queue_job_notify exist in pre-migration script to avoid error when migrating from 10.0 revision

[legalsylvain]

Hi,

Context

• upgrade queue_job module from version < 12.0. (in my case 10.0)
• the database doesn't have the trigger queue_job_notify on the table queue_job.

Rational

• For the time being, the post-migration 12.0.1.0.1/post-migration.py check if the trigger exist, and if not, create it. (via post_init_hook function)
• the pre-migration script 12.0.2.2.0/pre-migration.py consider that the trigger exist and disable it. (and enable it after...)

The problem is that all the pre-migration scripts are executed before all the post migration scripts.
So the pre-migration script 12.0.2.2.0 is executed before the trigger is created.

The problem is present since #333 has been merged. CC : authors and reviewers : @etobella, @StefanRijnhart, @simahawk, @guewen

Solution

• create the trigger in a pre-migration script.

Log after the PR

INFO odoo.modules.migration: module queue_job: Running migration [>12.0.1.0.1] pre-migration
INFO odoo.addons.queue_job.hooks.post_init_hook: Create queue_job_notify trigger
INFO odoo.modules.migration: module queue_job: Running migration [>12.0.2.1.0] pre-migration
INFO odoo.modules.migration: module queue_job: Running migration [>12.0.2.2.0] pre-migration
INFO odoo.modules.registry: module queue_job: creating or updating database tables
INFO odoo.models: Actual recompute of field ir.model.fields.related_field_id for 34 recs.
INFO odoo.models: Actual recompute of field ir.model.fields.relation_field_id for 33 recs.
INFO odoo.modules.loading: loading queue_job/security/security.xml
INFO odoo.models: Actual recompute of field res.users.share for 1 recs.
INFO odoo.models: Actual recompute of field res.partner.partner_share for 1 recs.
INFO odoo.modules.loading: loading queue_job/security/ir.model.access.csv
INFO odoo.modules.loading: loading queue_job/views/queue_job_views.xml
INFO odoo.modules.loading: loading queue_job/data/queue_data.xml
INFO odoo.models: Actual recompute of field queue.job.channel.complete_name for 1 recs.
INFO odoo.models: Actual recompute of field queue.job.channel.complete_name for 3 recs.
INFO odoo.models: Actual recompute of field queue.job.function.channel for 8 recs.
INFO odoo.modules.loading: loading queue_job/data/queue_job_function_data.xml
INFO odoo.models: Actual recompute of field queue.job.function.channel for 1 recs.
INFO odoo.models: Actual recompute of field queue.job.function.name for 1 recs.
INFO odoo.modules.migration: module queue_job: Running migration [12.0.1.0.1>] post-migration
INFO odoo.modules.migration: module queue_job: Running migration [12.0.2.0.0>] post-migration
INFO odoo.modules.migration: module queue_job: Running migration [12.0.2.3.0>] post-migration
INFO post-migration: Computing exception name for failed jobs

[StefanRijnhart]

If I understand, you encounter a postgresql error complaining that this trigger does not exist when it's being disabled in the pre-script. That trigger is disabled on purpose in the pre-stage to prevent the performance impact on table operations that it has. To force create it in another pre-script seems a bit convulated. Can you make it so that it is checked in 12.0.2.2.0/pre-migration.py if the trigger exists before it is disabled?

[legalsylvain]

Hi stefan, thanks for your review.

If I understand, you encounter a postgresql error complaining that this trigger does not exist when it's being disabled in the pre-script

indeed.

To force create it in another pre-script seems a bit convulated

I didn't added code, I just moved code from post-migration to pre-migration to be executed in the correct order. (check the diff in the file : queue_job/migrations/12.0.1.0.1/post-migration.py).

I mean : in the current implementation :
step x : in 12.0.2.2.0/pre-migration.py handle the trigger. here.
step y (executed after the step x) : in 12.0.1.0.1/post-migration.py ensure that the trigger exists creating it, if absent. here.

That is not consistent. Don't you think ?

[StefanRijnhart]

Ah I see that the trigger is disabled and then enabled again in the same script. In that case, it makes sense to ensure that the trigger exists beforehand. However, in https://github.com/OCA/queue/blob/12.0/queue_job/migrations/12.0.2.1.0/pre-migration.py there is another mass operation on the table, so unless you disable the trigger there as well, you might introduce a performance regression.

[legalsylvain]

Hi stefan.

Thank you for taking the time to discuss this topic.

However, in https://github.com/OCA/queue/blob/12.0/queue_job/migrations/12.0.2.1.0/pre-migration.py there is another mass operation on the table, so unless you disable the trigger there as well, you might introduce a performance regression.

Well,

• if trigger doesn't exist, the migration fails for the time being (since [12.0] Backport of 13.0 multiple improvements #333)
• if trigger exists, the script 12.0.2.1.0/pre-migration.py is already slow.
  So, not sure I introduce a performance regression.

however, I added an extra commit to handle that case : 6e162a4

Do you think it's OK ?

[StefanRijnhart]

@legalsylvain true! Thanks for the update.

[pedrobaeza]

I have been hit by this right now. Merging directly as the bot doesn't give any value in this case.