-
Notifications
You must be signed in to change notification settings - Fork 0
[FIX] keep context #1
base: 11.0
Are you sure you want to change the base?
Changes from 1 commit
c761ca2
4fc4f80
4e32fd4
f45ce59
4f73733
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,11 +5,14 @@ | |
| import functools | ||
| import hashlib | ||
| import logging | ||
| import json | ||
| import yaml | ||
| import uuid | ||
| import sys | ||
| from datetime import datetime, timedelta | ||
|
|
||
| import odoo | ||
| from odoo.tools.safe_eval import safe_eval | ||
|
|
||
| from .exception import (NoSuchJobError, | ||
| FailedJobError, | ||
|
|
@@ -55,14 +58,15 @@ class DelayableRecordset(object): | |
|
|
||
| def __init__(self, recordset, priority=None, eta=None, | ||
| max_retries=None, description=None, channel=None, | ||
| identity_key=None): | ||
| identity_key=None, keep_context=False): | ||
| self.recordset = recordset | ||
| self.priority = priority | ||
| self.eta = eta | ||
| self.max_retries = max_retries | ||
| self.description = description | ||
| self.channel = channel | ||
| self.identity_key = identity_key | ||
| self.keep_context = keep_context | ||
|
|
||
| def __getattr__(self, name): | ||
| if name in self.recordset: | ||
|
|
@@ -87,7 +91,9 @@ def delay(*args, **kwargs): | |
| eta=self.eta, | ||
| description=self.description, | ||
| channel=self.channel, | ||
| identity_key=self.identity_key) | ||
| identity_key=self.identity_key, | ||
| keep_context=self.keep_context | ||
| ) | ||
| return delay | ||
|
|
||
| def __str__(self): | ||
|
|
@@ -297,6 +303,7 @@ def _load_from_db_record(cls, job_db_record): | |
| if stored.company_id: | ||
| job_.company_id = stored.company_id.id | ||
| job_.identity_key = stored.identity_key | ||
| job_.keep_context = stored.context or {} | ||
| return job_ | ||
|
|
||
| def job_record_with_same_identity_key(self): | ||
|
|
@@ -311,7 +318,7 @@ def job_record_with_same_identity_key(self): | |
| @classmethod | ||
| def enqueue(cls, func, args=None, kwargs=None, | ||
| priority=None, eta=None, max_retries=None, description=None, | ||
| channel=None, identity_key=None): | ||
| channel=None, identity_key=None, keep_context=None): | ||
| """Create a Job and enqueue it in the queue. Return the job uuid. | ||
|
|
||
| This expects the arguments specific to the job to be already extracted | ||
|
|
@@ -324,7 +331,8 @@ def enqueue(cls, func, args=None, kwargs=None, | |
| new_job = cls(func=func, args=args, | ||
| kwargs=kwargs, priority=priority, eta=eta, | ||
| max_retries=max_retries, description=description, | ||
| channel=channel, identity_key=identity_key) | ||
| channel=channel, identity_key=identity_key, | ||
| keep_context=keep_context) | ||
| if new_job.identity_key: | ||
| existing = new_job.job_record_with_same_identity_key() | ||
| if existing: | ||
|
|
@@ -355,7 +363,8 @@ def db_record_from_uuid(env, job_uuid): | |
| def __init__(self, func, | ||
| args=None, kwargs=None, priority=None, | ||
| eta=None, job_uuid=None, max_retries=None, | ||
| description=None, channel=None, identity_key=None): | ||
| description=None, channel=None, | ||
| identity_key=None, keep_context=False): | ||
| """ Create a Job | ||
|
|
||
| :param func: function to execute | ||
|
|
@@ -381,6 +390,8 @@ def __init__(self, func, | |
| as argument) | ||
| :param env: Odoo Environment | ||
| :type env: :class:`odoo.api.Environment` | ||
| :param keep_context: Determine if the current context should be restored | ||
| :type keep_context: :bool or list | ||
| """ | ||
| if args is None: | ||
| args = () | ||
|
|
@@ -397,6 +408,7 @@ def __init__(self, func, | |
|
|
||
| recordset = func.__self__ | ||
| env = recordset.env | ||
| self.keep_context = keep_context | ||
| self.model_name = recordset._name | ||
| self.method_name = func.__name__ | ||
| self.recordset = recordset | ||
|
|
@@ -500,6 +512,10 @@ def store(self): | |
| } | ||
|
|
||
| dt_to_string = odoo.fields.Datetime.to_string | ||
| context = {} | ||
| if self.keep_context: | ||
| context = self.env.context.copy() | ||
| vals.update({"context": json.dumps(context)}) | ||
| if self.date_enqueued: | ||
| vals['date_enqueued'] = dt_to_string(self.date_enqueued) | ||
| if self.date_started: | ||
|
|
@@ -516,6 +532,9 @@ def store(self): | |
| db_record.write(vals) | ||
| else: | ||
| date_created = dt_to_string(self.date_created) | ||
| # We store the original context used at import on create | ||
| ctx = self.env.context.copy() or '{}' | ||
| vals.update({'original_context': json.dumps(ctx) or ''}) | ||
| # The following values must never be modified after the | ||
| # creation of the job | ||
| vals.update({'uuid': self.uuid, | ||
|
|
@@ -532,14 +551,40 @@ def store(self): | |
| if self.channel: | ||
| vals.update({'channel': self.channel}) | ||
|
|
||
| self.env[self.job_model_name].sudo().create(vals) | ||
| job = self.env[self.job_model_name].sudo().create(vals) | ||
|
||
|
|
||
| def db_record(self): | ||
| return self.db_record_from_uuid(self.env, self.uuid) | ||
|
|
||
| def _get_abs_context(self, original_ctx, ctx): | ||
| try: | ||
| import_ctx = json.loads(original_ctx) | ||
| current_ctx = json.loads(ctx) | ||
| except Exception as e: | ||
| _logger.error("\n\nERROR CONTEXT JSON CONVERSION: %s\n\n" % e) | ||
thomaspaulb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return self.env.context.copy() | ||
| else: | ||
| if isinstance(import_ctx, dict) and isinstance(current_ctx, dict): | ||
| import_ctx.update(current_ctx) | ||
| return import_ctx | ||
| return self.env.context.copy() | ||
|
|
||
| def _get_record_context(self): | ||
| """ | ||
| Get the context to execute the job | ||
| """ | ||
| ctx = self._get_abs_context(self.db_record().original_context, | ||
| self.db_record().context) | ||
| if self.company_id: | ||
| ctx.update({'allowed_company_ids': [self.company_id]}) | ||
| if self.uuid: | ||
| ctx.update({"job_uuid": self.uuid}) | ||
| return ctx | ||
|
|
||
| @property | ||
| def func(self): | ||
| recordset = self.recordset.with_context(job_uuid=self.uuid) | ||
| context = self._get_record_context() | ||
| recordset = self.recordset.with_context(**context) | ||
| recordset = recordset.sudo(self.user_id) | ||
| return getattr(recordset, self.method_name) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,3 +2,4 @@ | |
| from . import test_runner_runner | ||
| from . import test_json_field | ||
| from . import test_model_job_channel | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,7 @@ def test_batch(self): | |
| batch = self.env['queue.job.batch'].get_new_batch('TEST') | ||
| self.assertFalse(batch.job_ids) | ||
| model = self.env['test.queue.job'].with_context( | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @thomaspaulb the culprit was such line which made the conversion have issues when we assign batch record instead of the batch.id
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey, I think we were going about this totally the wrong way, because we did not understand the original PR. Instead of using a text or JSON field, they're actually (JSON-)encoding the original context in a variable Then instead of using So this then reinstates the context keys that were saved in that variable, one by one. I guess if that works, then it avoids 1) Deviating from the other PR and 2) spending time getting our own JSON conversion right The only comment on the current original PR was that by default it does not keep context, and most reviewers want it to keep context by default, and I agree with that too
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It already takes care of properly encoding and decoding Odoo recordsets: https://github.com/OCA/queue/blob/11.0/queue_job/fields.py#L31 |
||
| job_batch=batch | ||
| job_batch=batch.id | ||
| ) | ||
| job_1 = model.with_delay().testing_method() | ||
| self.assertEqual(job_1.db_record().state, 'pending') | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KKamaa I don't understand in which cases this would be different from
context.To me the code looks like this:
What am I missing? Can you give an example of a situation where
contextwould end up different fromoriginal_context?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thomaspaulb about the field original_context I added it to just show how the context we are using changes in execution of the queue job. When we create the queue job we have the entire import context, however when we start executing it the context is lost/modified. So I save the execution from the beginning in original_context field, then from there use what was saved and current context execution the update to use when executing the queue.job so:
{'params': {action: 23}} + {'lang': Fr_fr'} = {"params": {action: 23}, "lang': "Fr_fr"} for exampleabout the astor lib wasn't that successful with it and changed the part which was having issues. So if context contained a record say
job.batch(1,)it was hard in conversion. So I changed where job_batch was being assigned the record class instead of the id. I had added samples on the issue.So maybe we can just use context alone buy converting it the update it with current context then save again until the queue job is done. Just did a separate to have a proper sample test of the import working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh i get it... I think.
So in the original context you have things like 'lang' and 'invoice_type' which are saved to context
Then in the queue job execution some things are added such as 'job_uuid' (anything else?)
So if you combine both, then it should be good.