Skip to content

[FIX/ADD] configuration options#86

Merged
guewen merged 2 commits intoOCA:11.0from
nilshamerlinck:11.0_-_queue_job_-_configuration_options
Jul 30, 2018
Merged

[FIX/ADD] configuration options#86
guewen merged 2 commits intoOCA:11.0from
nilshamerlinck:11.0_-_queue_job_-_configuration_options

Conversation

@nilshamerlinck
Copy link
Copy Markdown
Contributor

@nilshamerlinck nilshamerlinck commented Jul 26, 2018

  1. 652911b re-introduced an harcoded http://localhost ;-)
  2. Port of [ADD] configurable db host and port connector#248

@coveralls
Copy link
Copy Markdown

coveralls commented Jul 26, 2018 •
edited
Loading

Coverage Status

Coverage decreased (-0.5%) to 78.287% when pulling c01ddf8 on nilshamerlinck:11.0_-queue_job-_configuration_options into ea36bbf on OCA:11.0.

@guewen
Copy link
Copy Markdown
Member

guewen commented Jul 27, 2018

652911b re-introduced an harcoded http://localhost ;-)

Oops, sorry 😑

guewen
guewen approved these changes Jul 27, 2018
View reviewed changes
Copy link
Copy Markdown
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.

LGTM

@guewen
Copy link
Copy Markdown
Member

guewen commented Jul 27, 2018

Only a few lint from travis to fix:

queue_job/jobrunner/runner.py:29:80: E501 line too long (96 > 79 characters)
queue_job/jobrunner/runner.py:33:80: E501 line too long (94 > 79 characters)
queue_job/jobrunner/runner.py:34:80: E501 line too long (89 > 79 characters)

simahawk
simahawk approved these changes Jul 27, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

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

LG, I'd replace a var name, but that's it :)
tnx for the change!

Comment thread queue_job/jobrunner/runner.py
def _connection_info_for(db_name):
db_or_uri, connection_info = odoo.sql_db.connection_info_for(db_name)

for p in ('host', 'port'):
Copy link
Copy Markdown
Contributor

@simahawk simahawk Jul 27, 2018

Choose a reason for hiding this comment

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

s/p/[key|param]

Copy link
Copy Markdown
Contributor Author

@nilshamerlinck nilshamerlinck Jul 27, 2018

Choose a reason for hiding this comment

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

If you don't mind, I will keep it as it to keep it consistent with the same piece of code that is in the other versions of the module.

Copy link
Copy Markdown
Contributor

@simahawk simahawk Jul 27, 2018

Choose a reason for hiding this comment

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

yeah, sorry, I missed the original code 😅 👍

@nilshamerlinck nilshamerlinck force-pushed the 11.0_-_queue_job_-_configuration_options branch from a96e0de to c01ddf8 Compare July 27, 2018 12:55
@guewen guewen merged commit 069456f into OCA:11.0 Jul 30, 2018
@whulshof
Copy link
Copy Markdown

whulshof commented Aug 2, 2018

Could this merge cause queue to not start anymore? I took the latest 10.0 merge and deployed it in my test-environment and now no jobs will start anymore. In the log the job runner gives the usual acknowledgement it runs on the db etc.
Should I change any configuration to get it running again? Which I would think is not to be expected, but anyhow.

@nilshamerlinck
Copy link
Copy Markdown
Contributor Author

nilshamerlinck commented Aug 2, 2018

Hi @whulshof, the PR for 10.0 is #77

Any error message? Are you using a connection pooler (pgbouncer)?

@whulshof
Copy link
Copy Markdown

whulshof commented Aug 2, 2018 via email

@sbidoul
Copy link
Copy Markdown
Member

sbidoul commented Aug 2, 2018

A common cause for jobs not starting is jobs in remaining in state enqueued or started when the server has not been shut down properly.

@whulshof
Copy link
Copy Markdown

whulshof commented Aug 2, 2018 via email

@lmignon
Copy link
Copy Markdown
Contributor

lmignon commented Aug 2, 2018

@whulshof do you have more than 1 database visible by your Odoo instance? If it's the case, can you retry with only 1 database visible by odoo?

@whulshof
Copy link
Copy Markdown

whulshof commented Aug 2, 2018 via email

@whulshof
Copy link
Copy Markdown

whulshof commented Aug 2, 2018 via email

@repodevs repodevs mentioned this pull request Nov 2, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simahawk simahawk simahawk approved these changes

@guewen guewen guewen approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@nilshamerlinck @coveralls @guewen @whulshof @sbidoul @lmignon @simahawk