Skip to content

Conversation

@eantones
Copy link
Contributor

No description provided.

Guewen Baconnier and others added 30 commits October 16, 2020 18:33
Instead of `ir.module.module`. Simplifies as we never hit the database nor have
to maintain a cache.

Also, it fixes an issue when we run tests directly at the installation of a
module, during the tests the previous implementation was considering the module
as uninstalled instead of installed.

Change proposed @bealdav. Thanks!

Fixes OCA#85
This context manager allows to execute a job function synchronously when it
should normally have been delayed in a asynchronous job.

This is really useful for writing tests that check the flow of a
synchronisation without having to deal with the jobs execution mechanisms.

It is used in the OCA/connector-magento project and ought to be used by other
projects.
This Postgres feature is invaluable when dealing with synchronisations,
especially when importing concurrently records from a system.

When we export a record, we are able to acquire a lock on the exported record
to prevent 2 jobs to export it at the same time. This is different when we
import a record for the first time and with several jobs running in parallel,
chances are high that 2 jobs will import the same record at the same moment.

The Postgres advisory lock comes handy there for they allow to acquire an
application lock.  Usually we'll acquire the lock at the beginning of an import
(beginning of ``Importer.run()``) and we'll throw a ``RetryableJobError`` if
the lock cannot be acquired so the job is retried later. The lock will remain
in place until the end of the transaction.

Example:
 - Job 1 imports Partner A
 - Job 2 imports Partner B
 - Partner A has a category X which happens not to exist yet
 - Partner B has a category X which happens not to exist yet
 - Job 1 import category X as a dependency
 - Job 2 import category X as a dependency

Since both jobs are executed concurrently, they both create a record for category X.
With this lock:

 - Job 1 imports Partner A, it puts a lock for this partner
 - Job 2 imports Partner B, it puts a lock for this partner
 - Partner A has a category X which happens not to exist yet
 - Partner B has a category X which happens not to exist yet
 - Job 1 import category X as a dependency, it puts a lock for this category
 - Job 2 import category X as a dependency, try to put a lock but can't, Job 2
   is retried later, and when it is retried, it sees the category X created by
   Job 1

See http://topopps.com/implementing-postgres-advisory-locks/ for the article
where I learned about the computation of the hash for this purpose.
It is no longer a context manager, because we would expect the lock to be
released at the end of the 'with' statement but it lasts until the end of the
transaction.
Commit c97ebbd was a frontport from 7.0.
However, openerp.sql_db.dsn was changed in 8.0 in that it now returns a
tuple. Extract the db_name from the returned tuple.
The first item contains the name of the DB such as 'odoo_db' and the
second item contains 'user=gbaconnier dbname=odoo_db' which is what
expects psycopg2.connect.
This is a temporary fix. In version 4.0,
dbfilter will be completely ignored by connector.
When PG is localized, error messages are not ascii and jobs are not postponed. Instead they are failed with a 'unicode decode error'.

Note: even if PG ```lc_messages``` is set to en_us.utf8 on a localized system, the error message prefixes (such as ```DETAILS:```) are still localized.
This is a safeguard to prevent someone to write arbitrary code in jobs.
Builtin types and datetime/timedelta are allowed in job arguments, and a
new function 'whitelist_unpickle_global' allows to register new objects
if needed.
didierdonze and others added 7 commits October 16, 2020 18:45
Currently translated at 99.1% (112 of 113 strings)

Translation: queue-13.0/queue-13.0-queue_job
Translate-URL: https://translation.odoo-community.org/projects/queue-13-0/queue-13-0-queue_job/de/
Context
-------

The queue_job module adds a new type of field, with the type
"job_serialized", which is close to the base "serialized" field,
adding support of serialization for recordsets and a couple of common
types such as datetime and date.

This field type is used in several fields of the "queue.job" model.

In "ir.model.fields", each field is related to a "ttype", which is
a selection containing every types of fields supported by the current
database.

The "job_serialized" field was never registered as a possible value for
the ttype selection field. It seems it had never been an issue before
Odoo 13.0. With Odoo 13.0, we may have this error on upgrades:

    INFO sample odoo.modules.loading: 297 modules loaded in 529.51s, 0 queries
    INFO sample odoo.addons.base.models.ir_model: Deleting 4@ir.model.fields.selection (base.selection__ir_model_fields__ttype__job_serialized)
    ERROR sample odoo.sql_db: bad query: UPDATE "ir_model_fields" SET "ttype"=NULL WHERE "ttype"='job_serialized'
    ERROR: null value in column "ttype" violates not-null constraint
    DETAIL:  Failing row contains (4296, record_ids, null, queue.job, null, null, null, 266, Record, null, null, t, null, null, f, t, f, f, null, base, null, null, t, null, null, null, null, null, t, null, null, 1, 2020-08-11 13:03:09.913127, null, null).

    2020-08-11 13:10:53,168 167 WARNING sample odoo.modules.loading: Transient module states were reset
    2020-08-11 13:10:53,170 167 ERROR sample odoo.modules.registry: Failed to load registry
    Traceback (most recent call last):
      File "/odoo/src/odoo/modules/registry.py", line 86, in new
        odoo.modules.load_modules(registry._db, force_demo, status, update_module)
      File "/odoo/src/odoo/modules/loading.py", line 472, in load_modules
        env['ir.model.data']._process_end(processed_modules)
      File "/odoo/src/odoo/addons/base/models/ir_model.py", line 1990, in _process_end
        record.unlink()
      File "/odoo/src/odoo/addons/base/models/ir_model.py", line 1208, in unlink
        self.env.cr.execute(query, [selection.value])
      File "/odoo/src/odoo/sql_db.py", line 168, in wrapper
        return f(self, *args, **kwargs)
      File "/odoo/src/odoo/sql_db.py", line 245, in execute
        res = self._obj.execute(query, params)
    psycopg2.IntegrityError: null value in column "ttype" violates not-null constraint
    DETAIL:  Failing row contains (4296, record_ids, null, queue.job, null, null, null, 266, Record, null, null, t, null, null, f, t, f, f, null, base, null, null, t, null, null, null, null, null, t, null, null, 1, 2020-08-11 13:03:09.913127, null, null).

Explanation
-----------

In 13.0, a model "ir.model.fields.selection" storing the allowed
selection values for a field has been added
(odoo/odoo@7593b88).

When the "job_serialized" fields for queue_job are created, they are
properly created in "ir.model.fields" with "job_serialized" as "ttype".
It creates the entry in "ir.model.fields.selection".

When we update the module, if it does not find "job_serialized" as a
possible value for IrModelFields.ttype, it unlinks the entry in
"ir.model.fields.selection". When this happens, an override of the
"unlink()" method in "ir.model.fields.selection" forces a NULL value in
the selection fields (here IrModelFields.ttype).

Fixes: OCA#227
@OCA-git-bot
Copy link
Contributor

Hi @guewen,
some modules you are maintaining are being modified, check this out!

@eantones eantones mentioned this pull request Oct 16, 2020
9 tasks
Copy link
Member

@guewen guewen left a comment

Choose a reason for hiding this comment

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

Thanks! (reviewed migration commits, untested)

@simahawk simahawk mentioned this pull request Oct 21, 2020
3 tasks
Copy link
Member

@bealdav bealdav left a comment

Choose a reason for hiding this comment

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

The last commit implies very small diff with previous version

Selection_106

Just new required changes in v14.

LGTM

@OCA-git-bot
Copy link
Contributor

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

@guewen
Copy link
Member

guewen commented Nov 2, 2020

I'd like to take the chance of removing completely the decorators in 14.0 (started for 13.0 #274):

  • 12.0: deprecate decorators but show no warning when they are used
  • 13.0: deprecate decorators and show warnings when they are used
  • 14.0: entirely remove them, as we are starting migrations, it will be easy to tackle in the modules

@guewen
Copy link
Member

guewen commented Nov 2, 2020

Thanks for the migration BTW :)

@atchuthan
Copy link
Member

@eantones
When I tried to test this PR for queue_job in my local DB, I got a few strange warnings in the odoo log.

WARNING testv14 odoo.fields: Field queue.job.record_ids: unknown parameter '_base_type', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it
WARNING testv14 odoo.fields: Field queue.job.args: unknown parameter '_base_type', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it
WARNING testv14 odoo.fields: Field queue.job.kwargs: unknown parameter '_base_type', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it

@atchuthan
Copy link
Member

atchuthan commented Nov 4, 2020

Same warning in runbot log too https://runbot2-3.odoo-community.org/runbot/static/build/3448412-263-8a6f92/logs/job_20_test_all.txt

2020-11-04 06:41:14,707 159 INFO openerp_test odoo.modules.loading: updating modules list
2020-11-04 06:41:14,709 159 INFO openerp_test odoo.addons.base.models.ir_module: ALLOW access to module.update_list on [] to user system #1 via n/a
2020-11-04 06:41:15,486 159 INFO openerp_test odoo.addons.base.models.ir_module: ALLOW access to module.button_install on ['queue_job'] to user system #1 via n/a
2020-11-04 06:41:15,590 159 INFO openerp_test odoo.modules.loading: loading 22 modules...
2020-11-04 06:41:15,860 159 INFO openerp_test odoo.modules.loading: 22 modules loaded in 0.27s, 0 queries (+0 extra)
2020-11-04 06:41:15,863 159 INFO openerp_test odoo.modules.loading: loading 23 modules...
2020-11-04 06:41:15,863 159 INFO openerp_test odoo.modules.loading: Loading module queue_job (20/23)
2020-11-04 06:41:16,500 159 WARNING openerp_test odoo.fields: Field queue.job.record_ids: unknown parameter '_base_type', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it
2020-11-04 06:41:16,500 159 WARNING openerp_test odoo.fields: Field queue.job.args: unknown parameter '_base_type', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it
2020-11-04 06:41:16,500 159 WARNING openerp_test odoo.fields: Field queue.job.kwargs: unknown parameter '_base_type', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it
2020-11-04 06:41:16,542 159 INFO openerp_test odoo.modules.registry: module queue_job: creating or updating database tables
2020-11-04 06:41:17,529 159 INFO openerp_test odoo.modules.loading: loading queue_job/security/security.xml
2020-11-04 06:41:17,626 159 INFO openerp_test odoo.modules.loading: loading queue_job/security/ir.model.access.csv
2020-11-04 06:41:17,664 159 INFO openerp_test odoo.modules.loading: loading queue_job/views/queue_job_views.xml
2020-11-04 06:41:17,839 159 INFO openerp_test odoo.modules.loading: loading queue_job/data/queue_data.xml

for record in self:
msg = record._message_failed_job()
if msg:
record.message_post(body=msg, subtype="queue_job.mt_job_failed")

Choose a reason for hiding this comment

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

Should be changed to subtype_xmlid

Copy link

@CasVissers-360ERP CasVissers-360ERP left a comment

Choose a reason for hiding this comment

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

Got a traceback for wrong subtype usage while testing.

@CasVissers-360ERP
Copy link

Additional:
Field queue.job.record_ids: unknown parameter '_base_type', if this is an actual parameter you may want to override the method _valid_field_parameter on the relevant model in order to allow it

@sebastienbeau
Copy link
Member

Hi, I have rebase the work done here : #286
For queue job we avoid starting from a blank repo as most of the module are always migrated.
So we prefers to keep a common history between branch (easier for maintaining)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.