Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract queue_job module from connector #1

Merged
merged 767 commits into from Jan 6, 2017
Merged

Conversation

guewen
Copy link
Member

@guewen guewen commented Nov 2, 2016

  • Replace ConnectorSession by odoo.api.Environment
  • Rework job classes
  • Jobs are now applied on Model methods
  class MyModel(models.Model):
     _name = 'my.model'

     @api.multi
     @job
     def my_method(self, a, k=None):
         _logger.info('executed with a: %s and k: %s', a, k)


  class MyOtherModel(models.Model):
      _name = 'my.other.model'

      @api.multi
      def button_do_stuff(self):
          self.env['my.model'].with_delay().my_method('a', k=2)

See history on OCA/connector#212

sbidoul and others added 30 commits May 14, 2015 22:19
…channels

What we receive from the queue are channel names and that's all.
My original idea was to let applications auto-create temporary channels
which could be useful for sequential channels.
But this needs more thinking.
This NOTIFY is not necessary and makes the purge of the job queue very slow.
Thanks to @acsonelmi for the suggestion.
Odoo indeed adapts the psycopg2 type mappings to use strings for datetime.
…able

The presence of ODOO_CONNECTOR_CHANNELS is enough to make the runner
start in place of the workers.
Since the runner is now a thread in the Odoo main process,
such a situation should occur only with a pathologically wrong
configuration, so no need to worry about that... unless proven
otherwise ;)
So the channel module does not make assumptions on the type
of the eta field, so we use the odoo-ish way to get now in
a database-compatible format, so it is more easily testable.
The previous version was using a daemon thread, which as more or less
ok, except it could enqueue jobs while the server was stopping, and such
jobs would never start.
When the @job is registered and created in the database, it lookups its channel
and creates it if it don't exist yet.
@leorochael
Copy link
Contributor

@guewen: guewen#5 for the config file work.

@adrienpeiffer
Copy link
Contributor

@leorochael Of course. But in this case, these mutable objects are stored in the database when the job failed. So, I think that it's not really clean.

@guewen. For instance, I give a dictionary that contains all data necessary the create a new partner. When the job failed (for any reason) the dictionary (args, kwargs) of the delayed method contains other key
in the dictionary like message_followers_ids or something like that.

Personally, I just make a deep copy of the dictionary in the delayed method when the job is running.

The model name, method name, recordset ids, arguments and keyword
arguments must never ever be modified after the creation of a job.
This change fix a bug when a job with a mutable argument see the content
of the argument being modified during the execution of a job.
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling efddad9 on guewen:10.0-add-queue_job into ** on OCA:10.0**.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling efddad9 on guewen:10.0-add-queue_job into ** on OCA:10.0**.

@guewen
Copy link
Member Author

guewen commented Jan 2, 2017

@adrienpeiffer Nice catch! Thanks for the explanation, I could fix it in efddad9.

@leorochael Thanks for bringing this feature back. I added a comment on the pull request regarding the naming of the section options-queue_job.

@coveralls
Copy link

coveralls commented Jan 2, 2017

Coverage Status

Changes Unknown when pulling 3ee43e4 on guewen:10.0-add-queue_job into ** on OCA:10.0**.

@sbidoul
Copy link
Member

sbidoul commented Jan 2, 2017

And when guewen#5 is merged, I'll do a PR for the sequential channels feature.

In addition to the ODOO_CONNECTOR_CHANNELS environment variable, for
backward compatibility, we can configure the runner channels through
the ODOO_QUEUE_JOB_CHANNELS and in the odoo configuration file like this:

    [options-job_queue]
    channels = root:4

For backward compatibility, the section `[options-connector]` is also
accepted.

Moreover, it's now also possible to preload the module in the config
file, like:

    [options]
    server_wide_modules = web,web_kanban,queue_job

As such we can configure the job runner using only the Odoo config file.
Ignore whitespace around values, and tolerate missing entries that would
be caused by trailing commas or commented lines when the channel
configuration is provided through the Odoo configuration file.

Also, accept line breaks channel entry separators along with commas,
which make the configuration file more readable.
Copy link
Contributor

@adrienpeiffer adrienpeiffer left a comment

Choose a reason for hiding this comment

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

👍 Great work !

leorochael and others added 4 commits January 3, 2017 21:37
Or replace them with references to queue_job.

Also, rip off the barely useful modicum of backward compatibility.
Forward port config file setup from connector
@sbidoul
Copy link
Member

sbidoul commented Jan 6, 2017

@guewen here is the last piece of the puzzle: guewen#6

@guewen
Copy link
Member Author

guewen commented Jan 6, 2017

Thanks @sbidoul. I'm eager to see this merged :)

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b47c9a6 on guewen:10.0-add-queue_job into ** on OCA:10.0**.

1 similar comment
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling b47c9a6 on guewen:10.0-add-queue_job into ** on OCA:10.0**.

@sbidoul sbidoul merged commit 4722729 into OCA:10.0 Jan 6, 2017
@sbidoul
Copy link
Member

sbidoul commented Jan 6, 2017

Voilà @guewen. À toi l'honneur du tweet.

@blaggacao
Copy link

@guewen Congratulations! This is actually really exciting work. To give people like me a headstart, consider a possible use cases section in the readme? Like advertising it a little bit, in the words of a seasoned contributor?

@leorochael
Copy link
Contributor

We should consider adding a github-pages or ReadTheDocs auto-generated site next.

Also, we should probably port base_import_async (and perhaps move it into this repo) as an example of what can be done with queue_job.

yucer pushed a commit to elego/connector-redmine that referenced this pull request Mar 17, 2017
yucer pushed a commit to elego/connector-redmine that referenced this pull request Mar 22, 2017
eLBati pushed a commit to eLBati/connector-redmine that referenced this pull request Apr 23, 2018
[MIG] Rename manifest file

[MIG] Replace "openerp" ocurrences with "odoo"

[MIG] Do not use RedmineConnectorSession anymore

(issue #212)

- extend ConnectorEnvironment to host the redmine_cache that was
  previously extended in RedmineConnectorSession

- remove session parameter from connector.get_environment (the
  connector module takes the environment from the backend object
  now). the default language from the backend is not applied there,
  because the session object is not available anymore.

- Simplify RedmineModelBinder reusing the generalizations from of
  Binder. This way the use of the session property is not needed.

- Proxy the redmine_cache of RedmineImportSyncronizer to the one saved
  in the connector environment. The same happens for RedmineAdapter

- Simplify RedmineImportSynchronizer to avoid the use of session

- remove "session" parameter from: import_batch, import_record,
  mock_delay, get_environment, import_single_user_time_entries. It is
  not replaced with "env", because env parameter passing is also
  removed in the last version of odoo_connector. the environment is
  now taken from the backend_record...

- that's why some methods pass now the backend object instead of the
  backend_id. The backend is used now to get the environment, not at
  the opossite.

[MIG] replace `hr.analytic.timesheet` by `account.analytic.line`

- since v9 the model 'hr.analytic.timesheet` has been replaced by
  `account.analytic.line`:

  https://github.com/OCA/OpenUpgrade/blob/9.0/addons/hr_timesheet/migrations/9.0.1.0/post-migration.py#L17

- note that in v9 we need also a mapper, in order to set::

       is_timesheet = True

  during the import of the timesheet records

[MIG] Use queue_job module from OCA/queue

see: OCA/queue#1

[MIG] Renamed ImportSynchronizer to Importer

- see: OCA/connector#47

[MIG] import exception module

- the job exception types has been moved to the module: `queue_job`.

- They are still loaded / proxied by the module
  `connector.exception`.

- But somehow this requires that the module is correctly imported.

- It is better to use always import. See:
  http://effbot.org/zone/import-confusion.htm

- Import made similar to:

  OCA/connector#188 (comment)

[MIG] slight changes to the Binder

- Rename 'to_backend' to 'to_external'

- Rename 'to_odoo' to 'to_internal'

see: OCA/connector@fef7ee2

pep8 adjustments

[MIG] rename ImportSynchronizers that left

[MIG] fix hr_timesheet_sheet_form

- the `buttons` element taked as reference by the previous version
  does not exist anymore

- another position was selected so the result is the same

fix pylint suggestions

fix pyflakes errors
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet