search

Tecnativa / doodba

Base image for making the creation of customized Odoo environments a piece of cake
Apache License 2.0
432 stars 304 forks source link

[FIX][16.0] click-odoo-contrib so out of date it doesnt understand 16.0 #541

Closed theangryangel closed 1 year ago

theangryangel commented 1 year ago

The 16.0 Dockerfile is referencing an older fork of click-odoo-contrib, resulting in click-odoo-update not actually being functional out of the box on a 16.0 install, throwing the following error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/site-packages/click_odoo/env_options.py", line 191, in _invoke
    with self.environment_manager(
  File "/usr/local/lib/python3.8/contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "/usr/local/lib/python3.8/site-packages/click_odoo_contrib/update.py", line 277, in OdooEnvironmentWithUpdate
    ignore_addons.update(core_addons[odoo.release.series])
KeyError: '16.0'
Error: '16.0'

Plus 16.0 requires a newer version anyway due to https://github.com/acsone/click-odoo-contrib/pull/119

Reading https://github.com/acsone/click-odoo-contrib/pull/93, as far as I can tell this fork is no longer relevant?

ap-wtioit commented 1 year ago

The reported error is a bit strange, click-odoo-update is tested here https://github.com/Tecnativa/doodba/blob/master/tests/__init__.py#L286

Also it seems it succeeded in the last merged MR, https://github.com/Tecnativa/doodba/actions/runs/4124654022/jobs/7124190939:

 INFO:root:Subtest execution: test_smallest (tests.ScaffoldingCase.test_smallest) (command=('click-odoo-update',), PWD='/home/runner/work/doodba/doodba/tests/scaffoldings/smallest', ODOO_MINOR='16.0', DB_VERSION='14')
/home/runner/.cache/pypoetry/virtualenvs/doodba-n4CMHPQX-py3.11/lib/python3.11/site-packages/paramiko/transport.py:236: CryptographyDeprecationWarning: Blowfish has been deprecated
  "class": algorithms.Blowfish,
Creating smallest_odoo_run ... 

Creating smallest_odoo_run ... done
doodba INFO: Waiting until postgres is listening at db...
doodba INFO: Linking all addons from /opt/odoo/custom/src/addons.yaml in /opt/odoo/auto/addons
doodba INFO: Generating /opt/odoo/auto/odoo.conf file. Overriding any existing...
doodba INFO: Merging found configuration files in /opt/odoo/auto/odoo.conf
doodba INFO: Executing click-odoo-update
2023-02-08 14:15:41,314 1 INFO ? odoo: Odoo version 16.0 
2023-02-08 14:15:41,314 1 INFO ? odoo: Using configuration file at /opt/odoo/auto/odoo.conf 
2023-02-08 14:15:41,314 1 INFO ? odoo: addons paths: ['/opt/odoo/custom/src/odoo/odoo/addons', '/var/lib/odoo/addons/16.0', '/opt/odoo/auto/addons'] 
2023-02-08 14:15:41,314 1 INFO ? odoo: database: odoo@db:5432 
2023-02-08 14:15:41,360 1 INFO prod odoo.modules.loading: loading 1 modules... 
2023-02-08 14:15:41,535 1 INFO prod odoo.addons.base.models.ir_actions_report: Will use the Wkhtmltopdf binary at /usr/local/bin/wkhtmltopdf 
2023-02-08 14:15:41,821 1 INFO prod odoo.modules.loading: 1 modules loaded in 0.46s, 0 queries (+0 extra) 
2023-02-08 14:15:41,842 1 INFO prod odoo.modules.loading: updating modules list 
2023-02-08 14:15:41,845 1 INFO prod odoo.addons.base.models.ir_module: ALLOW access to module.update_list on [] to user __system__ #1 via n/a 
2023-02-08 14:15:42,642 1 INFO prod odoo.modules.loading: loading 8 modules... 
2023-02-08 14:15:42,760 1 INFO prod odoo.modules.loading: 8 modules loaded in 0.12s, 0 queries (+0 extra) 
2023-02-08 14:15:42,939 1 INFO prod odoo.modules.loading: Modules loaded. 
2023-02-08 14:15:42,943 1 INFO prod odoo.modules.registry: Registry loaded in 1.594s 
2023-02-08 14:15:43,092 1 INFO prod click_odoo_contrib.update: Database updated, new checksums stored

How can we improve the test to reproduce the error (before merging this)?

theangryangel commented 1 year ago

How can we improve the test to reproduce the error (before merging this)?

I’ll see what I can figure out, but it’ll be a day or two before I can circle back to this, unfortunately :(

ap-wtioit commented 1 year ago

I tried to reproduce the issue on our test servers but it seems since https://github.com/acsone/click-odoo-contrib/pull/86 we are using the newest click-odoo-contrib in our images.

yajo commented 1 year ago

This will break other things because https://github.com/acsone/click-odoo-contrib/pull/93 is needed. Please go there and vote for merging 😅

pedrobaeza commented 1 year ago

We don't have a manifest with active since years, and the case that makes it break seems very specific to l10n_fi. Or is there more cases?

theangryangel commented 1 year ago

The reported error is a bit strange, click-odoo-update is tested here https://github.com/Tecnativa/doodba/blob/master/tests/__init__.py#L286

It seems that click-odoo-update only looks at the addon list if it's been asked to ignore core addons. Makes sense. I've modified the test to add that scenario as well.

I'm assuming one of the tests is going to fail, but unfortunately I've now run out of time to look further today and I'll have to come back to this again tomorrow :(

This will break other things because acsone/click-odoo-contrib#93 is needed. Please go there and vote for merging sweat_smile

l10n_fi active was removed back in Nov 2020 under https://github.com/odoo/odoo/commit/6a6077b3136f4d0a931c85303b71a03e1c819e65

From a quick git grep active -- '**/__manifest__.py' of both odoo core and enterprise 16.0 I can see no manifests where active is still in the manifest.

From a similar quick search, I'm struggling to find any modules where auto_install is set to an empty list on either odoo core or enterprise.

Assuming I've fully understood the linked PR, from a correctness point of view, I'd agree it's probably something that should be fixed.

However, from a pragmatic point of view, I feel it's difficult to run into this scenario currently, and the PR might break something, but this is broken... Or am I being overly optimistic? :D

pedrobaeza commented 1 year ago

So this seems logical from a pragmatic point of view for not having to rebase the pull request branch, and the PR is stalled because it seems conflicting. I vote for go with this change fixing the red test.

ap-wtioit commented 1 year ago

The breaking test in 14.0 broke because of something unrelated:

 #17 1.535 Err:7 http://apt.postgresql.org/pub/repos/apt bullseye-pgdg InRelease
#17 1.536   The following signatures were invalid: BADSIG 7FCC7D46ACCC4CF8 PostgreSQL Debian Repository

Reproducing the issues in doodba tests would likely involve creating a database that has something to update. We have a test like that in our CI (involving building the image odoo with 100 commits behind, installing with the "old" image, rebuilding the image, and calling the update script with the new image) but i think this is overkill to test it in doodba (for us i just want to be extra safe).

As the MR only changes the image for 16.0 and active should no longer be used in manifests i also think it's best to merge this.

The commit removing the active in Odoo https://github.com/odoo/odoo/pull/62086/commits/b5fde28d56bfd84d8c6b167647570de2d41e5551 seems to be in Odoo starting with saas-14.1 so the workarround https://github.com/acsone/click-odoo-contrib/pull/93 should only be needed for Versions <15.0.

pedrobaeza commented 1 year ago

@theangryangel can you perform such changes to go ahead with this?

theangryangel commented 1 year ago

Changed I've made:

I hope that covers what was expected? Please do let me know if I've misunderstood :)

theangryangel commented 1 year ago

Thanks everyone for the input and time on this one <3

pedrobaeza commented 1 year ago

Thanks to you for digging till the resolution.