Skip to content

[14.0][FIX] queue_job: Add base_sparse_field dependency to prevent errors in test if addon not loaded. #443

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.

Sign up for GitHub

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

Merged
OCA-git-bot merged 1 commit into from
Aug 11, 2022
Merged

[14.0][FIX] queue_job: Add base_sparse_field dependency to prevent errors in test if addon not loaded. #443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion queue_job/README.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ Configuration
[options]
(...)
workers = 6
server_wide_modules = web,queue_job
server_wide_modules = web,queue_job,base_sparse_field
Copy link
Contributor

@simahawk simahawk Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

To my knowledge, this is NOT needed when the field is imported directly instead of using the patched odoo.fields.Serialized (like in queue_job).

Copy link
Member

@pedrobaeza pedrobaeza Aug 16, 2022

Choose a reason for hiding this comment

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

But I have shown you the capture with the error in our CI when we run the server without this:

imagen

Copy link
Contributor

@simahawk simahawk Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This problem might come from the fact that base_sparse_field is not available in your env (for whatever reason) or you are using an outdated version of queue_job... I don't know, can't judge from a screenshot 🤷‍♂️
We never had this problem on any of our instances or dev setup or CI setup. Even when we use --load=queue_job.

FTR this was fixed a looong time ago here 547538b and if you look at the change we removed the need for the server wide module. Hence it looks weird, at least 😉

Copy link
Member

@pedrobaeza pedrobaeza Aug 16, 2022

Choose a reason for hiding this comment

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

The code of queue_job is updated to latest one. Don't you think that not depending in the manifest on base_sparse_field is an error?

Copy link
Member

@pedrobaeza pedrobaeza Aug 16, 2022

Choose a reason for hiding this comment

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

And what's the problem with having such server wide module loaded?

Copy link
Contributor

@simahawk simahawk Aug 16, 2022

Choose a reason for hiding this comment

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

I'm arguing only about the server_wide_modules change here. The dependency change IS correct.

Copy link
Member

@pedrobaeza pedrobaeza Aug 16, 2022

Choose a reason for hiding this comment

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

But one is attached to the other: if you don't add it to the server wide modules, then it's not loaded at all. And I insist: is there any problem adding to the server wide modules that I'm not aware of?

Copy link
Contributor

@simahawk simahawk Aug 16, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

if you don't add it to the server wide modules, then it's not loaded at all

AFAIK This is not true. It was required WHEN we were using the patch.

AFAIK The server wide module IS NOT needed. If you want to recommend it for certain cases, fine, but if you want to state in the docs that is required, to me is wrong. Unless proven otherwise. This is my point.

Copy link
Member

@pedrobaeza pedrobaeza Aug 16, 2022

Choose a reason for hiding this comment

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

Tested and indeed the server wide is not needed. @victoralmau please do a PR in 14.0 for removing such change in the documentation.


(...)
[queue_job]
Expand Down
2 changes: 1 addition & 1 deletion queue_job/__manifest__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
"website": "https://github.com/OCA/queue",
"license": "LGPL-3",
"category": "Generic Modules",
"depends": ["mail"],
"depends": ["mail", "base_sparse_field"],
"external_dependencies": {"python": ["requests"]},
"data": [
"security/security.xml",
Expand Down
2 changes: 1 addition & 1 deletion queue_job/readme/CONFIGURE.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
[options]
(...)
workers = 6
server_wide_modules = web,queue_job
server_wide_modules = web,queue_job,base_sparse_field

(...)
[queue_job]
Expand Down
4 changes: 2 additions & 2 deletions queue_job/static/description/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta name="generator" content="Docutils 0.15.1: http://docutils.sourceforge.net/" />
<meta name="generator" content="Docutils: http://docutils.sourceforge.net/" />
<title>Job Queue</title>
<style type="text/css">

Expand Down Expand Up @@ -462,7 +462,7 @@ <h1><a class="toc-backref" href="#id4">Configuration</a></h1>
<span class="k">[options]</span>
<span class="na">(...)</span>
<span class="na">workers</span> <span class="o">=</span> <span class="s">6</span>
<span class="na">server_wide_modules</span> <span class="o">=</span> <span class="s">web,queue_job</span>
<span class="na">server_wide_modules</span> <span class="o">=</span> <span class="s">web,queue_job,base_sparse_field</span>

<span class="na">(...)</span>
<span class="k">[queue_job]</span>
Expand Down