Skip to content

[MIG] database_cleanup: Migration to 14.0 - #1990

Closed
StephaneMangin wants to merge 44 commits into
OCA:14.0from
camptocamp:14.0-mig-database_cleanup
Closed

[MIG] database_cleanup: Migration to 14.0#1990
StephaneMangin wants to merge 44 commits into
OCA:14.0from
camptocamp:14.0-mig-database_cleanup

Conversation

@StephaneMangin

@StephaneMangin StephaneMangin commented Jan 12, 2021

Copy link
Copy Markdown

Comments and changes from #1885 are taken in account.

@StephaneMangin StephaneMangin mentioned this pull request Jan 12, 2021
26 tasks
@StephaneMangin
StephaneMangin marked this pull request as draft January 12, 2021 13:40
@StephaneMangin
StephaneMangin force-pushed the 14.0-mig-database_cleanup branch 9 times, most recently from 9e1a0e4 to 007979e Compare January 14, 2021 15:20
@i-vyshnevska

Copy link
Copy Markdown
Member

@StephaneMangin you need to check the tests

@StefanRijnhart

Copy link
Copy Markdown
Member

For the record, #1885 is now finalized.

@StephaneMangin

Copy link
Copy Markdown
Author

For the record, #1885 is now finalized.

Thanks @StefanRijnhart , i'll check that as soon as i can ^^

@max3903 max3903 added this to the 14.0 milestone Feb 4, 2021
@gustavovalverde

Copy link
Copy Markdown
Member

Hi @StephaneMangin, please let me know if you will continue migrating this one, or if you might need help to do it so I can take over the following week.

@StephaneMangin

Copy link
Copy Markdown
Author

Hi @StephaneMangin, please let me know if you will continue migrating this one, or if you might need help to do it so I can take over the following week.

Hi @gustavovalverde, thanx for the reminder. I'm a little bit busy right now. If it's urgent for you, let me know. I probably will take it apart in the following week to finish the work.

@leemannd
leemannd force-pushed the 14.0-mig-database_cleanup branch from 007979e to 202949d Compare March 19, 2021 14:27
@dcorio

dcorio commented Apr 7, 2021

Copy link
Copy Markdown

Hi, FYI: I've tested the PR and noticed that "read" permission only on the model is not enough for the users to create the various purge wizards.

@leemannd
leemannd force-pushed the 14.0-mig-database_cleanup branch from 6d48a3b to e9c2ea0 Compare May 31, 2021 08:17
@StephaneMangin
StephaneMangin force-pushed the 14.0-mig-database_cleanup branch 2 times, most recently from 1d3fe3a to ed0bc91 Compare September 13, 2021 15:13
@StephaneMangin
StephaneMangin marked this pull request as ready for review September 13, 2021 15:14
@StephaneMangin
StephaneMangin force-pushed the 14.0-mig-database_cleanup branch 3 times, most recently from 413b82c to 10de2e0 Compare September 20, 2021 10:52

@leemannd leemannd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Functionally does the work.
If there are missing points I think it should be mentioned and we could maybe consider merging it?

@StephaneMangin

StephaneMangin commented Sep 23, 2021

Copy link
Copy Markdown
Author

The model created to test the model cleaning behavior fails. Issue append while loading the module after the test have been succeded :

        # 2021-09-23 10:40:28,565 93 INFO odoodb_test odoo.modules.loading: Module database_cleanup loaded in 37.01s (incl. 36.22s test), 768 queries (+5536 test)
        # 2021-09-23 10:45:05,416 92 WARNING odoodb_test odoo.modules.loading: Transient module states were reset
        # 2021-09-23 10:45:05,416 92 ERROR odoodb_test odoo.modules.registry: Failed to load registry
        # 2021-09-23 10:45:05,416 92 CRITICAL odoodb_test odoo.service.server: Failed to initialize database `odoodb_test`.
        # Traceback (most recent call last):
        #   File "/odoo/src/odoo/service/server.py", line 1199, in preload_registries
        #     registry = Registry.new(dbname, update_module=update_module)
        #   File "/odoo/src/odoo/modules/registry.py", line 89, in new
        #     odoo.modules.load_modules(registry._db, force_demo, status, update_module)
        #   File "/odoo/src/odoo/modules/loading.py", line 451, in load_modules
        #     force, status, report, loaded_modules, update_module, models_to_check)
        #   File "/odoo/src/odoo/modules/loading.py", line 348, in load_marked_modules
        #     perform_checks=perform_checks, models_to_check=models_to_check
        #   File "/odoo/src/odoo/modules/loading.py", line 187, in load_module_graph
        #     model_names = registry.load(cr, package)
        #   File "/odoo/src/odoo/modules/registry.py", line 232, in load
        #     model = cls._build_model(self, cr)
        #   File "/odoo/src/odoo/models.py", line 588, in _build_model
        #     ModelClass._build_model_attributes(pool)
        #   File "/odoo/src/odoo/models.py", line 670, in _build_model_attributes
        #     child_class = pool[child_name]
        #   File "/odoo/src/odoo/modules/registry.py", line 177, in __getitem__
        #     return self.models[model_name]
        # KeyError: 'x_database.cleanup.test.model'

Recreating the model in tearDown does the job ... but why ?

@StephaneMangin
StephaneMangin force-pushed the 14.0-mig-database_cleanup branch from 10de2e0 to 6a24fbd Compare September 23, 2021 11:03
Rodrigo Macedo and others added 5 commits February 1, 2022 14:40
Currently translated at 100.0% (74 of 74 strings)

Translation: server-tools-12.0/server-tools-12.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-12-0/server-tools-12-0-database_cleanup/pt_BR/
Currently translated at 51.4% (38 of 74 strings)

Translation: server-tools-12.0/server-tools-12.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-12-0/server-tools-12-0-database_cleanup/zh_CN/
Currently translated at 100.0% (74 of 74 strings)

Translation: server-tools-12.0/server-tools-12.0-database_cleanup
Translate-URL: https://translation.odoo-community.org/projects/server-tools-12-0/server-tools-12-0-database_cleanup/zh_CN/
@StephaneMangin
StephaneMangin force-pushed the 14.0-mig-database_cleanup branch from ad7211d to b68617c Compare February 1, 2022 13:42
@matteoopenf

Copy link
Copy Markdown
Contributor

is it ready to merge?

@dreispt

dreispt commented Feb 6, 2022

Copy link
Copy Markdown
Member

/ocabot merge nobump

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 14.0-ocabot-merge-pr-1990-by-dreispt-bump-nobump, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Feb 6, 2022
Signed-off-by dreispt
@OCA-git-bot

Copy link
Copy Markdown
Contributor

@dreispt your merge command was aborted due to failed check(s), which you can inspect on this commit of 14.0-ocabot-merge-pr-1990-by-dreispt-bump-nobump.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

@StephaneMangin

Copy link
Copy Markdown
Author

is it ready to merge?

No not yet. We have an issue with the way the module drop tables, the relation finder and management is not efficient. This issue must be addressed before merging.

@matteoopenf

Copy link
Copy Markdown
Contributor

is it ready to merge?

No not yet. We have an issue with the way the module drop tables, the relation finder and management is not efficient. This issue must be addressed before merging.

ok, thanks for the information

@StephaneMangin

StephaneMangin commented Feb 7, 2022

Copy link
Copy Markdown
Author

is it ready to merge?

No not yet. We have an issue with the way the module drop tables, the relation finder and management is not efficient. This issue must be addressed before merging.

ok, thanks for the information

After local test, everything is fine, even with base_custom_info installed. Then I can't reproduce this Travis issue.
This module is functionnally ready to be merged.

$ odoo testdb-gen -i database_cleanup,base_custom_info
$ odoo pytest odoo/external-src/server-tools/database_cleanup
(...)
Running without demo data
2022/02/07 14:36:10 Waiting for: tcp://db:5432
2022/02/07 14:36:10 Connected to tcp://db:5432
/usr/local/lib/python3.7/dist-packages/psycopg2/__init__.py:144: UserWarning: The psycopg2 wheel package will be renamed from release 2.8; in order to keep installing from binary please use "pip install psycopg2-binary" instead. For details see: <http://initd.org/psycopg/docs/install.html#binary-install-from-pypi>.
  """)
2022-02-07 14:36:11,742 1 WARNING ? py.warnings: /usr/local/lib/python3.7/dist-packages/_pytest/compat.py:333: PytestDeprecationWarning: The TerminalReporter.writer attribute is deprecated, use TerminalReporter._tw instead at your own risk.
See https://docs.pytest.org/en/latest/deprecations.html#terminalreporter-writer for more information.
  return getattr(object, name, default)
 
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.7.3, pytest-5.4.3, py-1.5.3, pluggy-0.13.1
rootdir: /odoo
plugins: cov-2.11.1, odoo-0.6.0
collected 9 items                                                                                                                                                                                                 

odoo/external-src/server-tools/database_cleanup/tests/test_create_indexes.py .                                                                                                                              [ 11%]
odoo/external-src/server-tools/database_cleanup/tests/test_purge_columns.py .                                                                                                                               [ 22%]
odoo/external-src/server-tools/database_cleanup/tests/test_purge_data.py .                                                                                                                                  [ 33%]
odoo/external-src/server-tools/database_cleanup/tests/test_purge_menus.py .                                                                                                                                 [ 44%]
odoo/external-src/server-tools/database_cleanup/tests/test_purge_models.py .                                                                                                                                [ 55%]
odoo/external-src/server-tools/database_cleanup/tests/test_purge_modules.py .                                                                                                                               [ 66%]
odoo/external-src/server-tools/database_cleanup/tests/test_purge_properties.py ..                                                                                                                           [ 88%]
odoo/external-src/server-tools/database_cleanup/tests/test_purge_tables.py .                                                                                                                                [100%]

================================================================================================ warnings summary =================================================================================================
external-src/server-tools/database_cleanup/tests/test_create_indexes.py::TestCreateIndexesLine::test_deleted_index
  /odoo/src/addons/web_editor/controllers/main.py:221: DeprecationWarning: invalid escape sequence \d
    number_match = re.match('^(\d+)', url_segment)

external-src/server-tools/database_cleanup/tests/test_create_indexes.py::TestCreateIndexesLine::test_deleted_index
  /odoo/src/addons/web_editor/controllers/main.py:341: DeprecationWarning: invalid escape sequence \.
    excluded_url_matcher = re.compile("^(.+/lib/.+)|(.+import_bootstrap.+\.scss)$")

external-src/server-tools/database_cleanup/tests/test_create_indexes.py::TestCreateIndexesLine::test_deleted_index
  /odoo/src/addons/web_editor/models/ir_qweb.py:570: DeprecationWarning: invalid escape sequence \s
    return re.sub('\s+', ' ', text)

external-src/server-tools/database_cleanup/tests/test_create_indexes.py::TestCreateIndexesLine::test_deleted_index
  /odoo/src/addons/web_editor/models/assets.py:14: DeprecationWarning: invalid escape sequence \w
    _match_asset_file_url_regex = re.compile("^/(\w+)/(.+?)(\.custom\.(.+))?\.(\w+)$")

external-src/server-tools/database_cleanup/tests/test_purge_modules.py::TestCleanupPurgeLineModule::test_remove_to_upgrade_module
external-src/server-tools/database_cleanup/tests/test_purge_modules.py::TestCleanupPurgeLineModule::test_remove_to_upgrade_module
external-src/server-tools/database_cleanup/tests/test_purge_modules.py::TestCleanupPurgeLineModule::test_remove_to_upgrade_module
external-src/server-tools/database_cleanup/tests/test_purge_modules.py::TestCleanupPurgeLineModule::test_remove_to_upgrade_module
  <unknown>:19: DeprecationWarning: invalid escape sequence \

external-src/server-tools/database_cleanup/tests/test_purge_modules.py::TestCleanupPurgeLineModule::test_remove_to_upgrade_module
external-src/server-tools/database_cleanup/tests/test_purge_modules.py::TestCleanupPurgeLineModule::test_remove_to_upgrade_module
  <unknown>:18: DeprecationWarning: invalid escape sequence \

-- Docs: https://docs.pytest.org/en/latest/warnings.html
========================================================================================= 9 passed, 10 warnings in 3.53s ==========================================================================================

@matteoopenf

Copy link
Copy Markdown
Contributor

is it ready to merge?

No not yet. We have an issue with the way the module drop tables, the relation finder and management is not efficient. This issue must be addressed before merging.

ok, thanks for the information

After local test, everything is fine, even with base_custom_info installed. Then I can't reproduce this Travis issue. This module is functionnally ready to be merged.

I think at this time, Travis is not important but is important runboat, because Travis will be decommissioned.

@matteoopenf

Copy link
Copy Markdown
Contributor

is it ready to merge?

No not yet. We have an issue with the way the module drop tables, the relation finder and management is not efficient. This issue must be addressed before merging.

ok, thanks for the information

After local test, everything is fine, even with base_custom_info installed. Then I can't reproduce this Travis issue. This module is functionnally ready to be merged.

I think at this time, Travis is not important but is important runboat, because Travis will be decommissioned.

I made an errore runbot change to runboat, but travis not change

@sbidoul

sbidoul commented Feb 25, 2022

Copy link
Copy Markdown
Member

Note runboat does not run tests. In this case the test failure is likely to persist when we move to GitHub action.

@matteoopenf

Copy link
Copy Markdown
Contributor

are there any news about the travis error?



# Use post_install to get all models loaded more info: odoo/odoo#13458
@tagged("post_install", "-at_install")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think something may be wrong here.

Note that the normal tests fail before the post tests get started

But the database_cleanup test should be part of the post installation tests, because otherwise, not all modules are loaded (eg. base_custom_info is loaded as number 53) before database_cleanup is loaded (as number 22, so earlier) and database_cleanup will try to delete tables belonging to not-yet-loaded modules instead of deprecated modules and you get these errors.

So, I suspect that with Odoo 14.0, @tagged perhaps does not propagate anymore to child tests and we should define it on all tests specifically.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@GSLabIt

GSLabIt commented Aug 14, 2022

Copy link
Copy Markdown
Contributor

@StephaneMangin any chance to fix and rebase pr? Thx

<field name="state">code</field>
<field name="model_id" ref="database_cleanup.model_cleanup_purge_wizard_menu" />
<field name="code">
action = env.get('cleanup.purge.wizard').get_wizard_action()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
action = env.get('cleanup.purge.wizard').get_wizard_action()
action = env.get('cleanup.purge.wizard.menu').get_wizard_action()

the .menu part is missing.

@StephaneMangin

Copy link
Copy Markdown
Author

Superseeded by #2390

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.