[13.0] [MIG] database_cleanup by Rad0van · Pull Request #1885 · OCA/server-tools

Rad0van · 2020-08-15T18:33:10Z

Standard migration. Added _description to few classes to get rig of WARNING messages (not present in 12.0). Also changed broken translation in en.po that translated "Purge obsolete properties" as "Purge obsolete models".

Rad0van · 2020-08-15T19:49:39Z

This is strange - Travis CI fails for some reason. This does not happen in my environment, did also clean instance. @hbrunn could you please have a look? Is it me or Travis?

AaronHForgeFlow · 2020-08-16T10:37:30Z

Hi @Rad0van I was attempting to migrate on #1823 Did you check the improvements there? For me it is ok to continue the PR here as long you include the improve commented by @i-vyshnevska and 76f57df and 5758932 and 4a45ade

BTW, for reviewing it is best to create a separate commit for the black, isort changes.

Rad0van · 2020-08-16T11:40:12Z

Hi @Rad0van I was attempting to migrate on #1823 Did you check the improvements there? For me it is ok to continue the PR here as long you include the improve commented by @i-vyshnevska and 76f57df and 5758932 and 4a45ade

BTW, for reviewing it is best to create a separate commit for the black, isort changes.

@AaronHForgeFlow stupid me - forgot to check if anyboy else is working on this. My fault. Didn't see the one you did. But I see it is failing the same way my does :-) Does 4a45ade fix it?

Regarding black and isort changes - I follow the migration guide so I ususally do that. Must have forgotten this time.

So I see 2 options here - please advise which to choose:

I'll close my PR and you finish yours.
I will rework mine and include the fixes you mention.

What is your preferred option?

AaronHForgeFlow · 2020-08-16T12:23:21Z

Currently I don't have time to finish my PR so option 2 it's fine to me :)

Rad0van · 2020-08-16T14:34:13Z

OK, I will rework this one. Better in new PR. So closing this one.

Rad0van · 2020-08-16T17:06:47Z

@AaronHForgeFlow tried reworking int. Some patches were already present because I migrateda later. However the Travis still fails. Patch [4a45ade] didn't solve it - it still fails the same way it did for you. Do you have any idea as why that happens?

AaronHForgeFlow · 2020-08-16T18:00:59Z

It seems it is a CacheMiss Error. I have seen similar errors before but I really don't know how to solve this. Perhaps @hbrunn or @StefanRijnhart can help us here.

ERROR: TestDatabaseCleanup.test_database_cleanup_modules
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/api.py", line 745, in get
    value = self._data[field][record._ids[0]]
KeyError: 398
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1002, in __get__
    value = env.cache.get(record, self)
  File "/home/travis/odoo-13.0/odoo/api.py", line 751, in get
    raise CacheMiss(record, field)
odoo.exceptions.CacheMiss: ('ir.module.module(398,).views_by_module', None)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/models.py", line 5796, in process
    recs.mapped(field.name)
  File "/home/travis/odoo-13.0/odoo/models.py", line 5210, in mapped
    recs = recs._mapped_func(operator.itemgetter(name))
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in _mapped_func
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in <listcomp>
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5620, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1015, in __get__
    _("(Record: %s, User: %s)") % (record, env.uid),
odoo.exceptions.MissingError: ('Record does not exist or has been deleted.\n(Record: ir.module.module(398,), User: 1)', None)

Rad0van · 2020-08-16T18:46:24Z

It seems it is a CacheMiss Error. I have seen similar errors before but I really don't know how to solve this. Perhaps @hbrunn or @StefanRijnhart can help us here.

ERROR: TestDatabaseCleanup.test_database_cleanup_modules
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/api.py", line 745, in get
    value = self._data[field][record._ids[0]]
KeyError: 398
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1002, in __get__
    value = env.cache.get(record, self)
  File "/home/travis/odoo-13.0/odoo/api.py", line 751, in get
    raise CacheMiss(record, field)
odoo.exceptions.CacheMiss: ('ir.module.module(398,).views_by_module', None)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/models.py", line 5796, in process
    recs.mapped(field.name)
  File "/home/travis/odoo-13.0/odoo/models.py", line 5210, in mapped
    recs = recs._mapped_func(operator.itemgetter(name))
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in _mapped_func
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in <listcomp>
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5620, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1015, in __get__
    _("(Record: %s, User: %s)") % (record, env.uid),
odoo.exceptions.MissingError: ('Record does not exist or has been deleted.\n(Record: ir.module.module(398,), User: 1)', None)

Oh I thought you solved it somehow. Ok, will try to investigate more.

CasVissers-360ERP · 2020-08-18T14:15:35Z

It seems it is a CacheMiss Error. I have seen similar errors before but I really don't know how to solve this. Perhaps @hbrunn or @StefanRijnhart can help us here.

ERROR: TestDatabaseCleanup.test_database_cleanup_modules
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/api.py", line 745, in get
    value = self._data[field][record._ids[0]]
KeyError: 398
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1002, in __get__
    value = env.cache.get(record, self)
  File "/home/travis/odoo-13.0/odoo/api.py", line 751, in get
    raise CacheMiss(record, field)
odoo.exceptions.CacheMiss: ('ir.module.module(398,).views_by_module', None)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/models.py", line 5796, in process
    recs.mapped(field.name)
  File "/home/travis/odoo-13.0/odoo/models.py", line 5210, in mapped
    recs = recs._mapped_func(operator.itemgetter(name))
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in _mapped_func
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in <listcomp>
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5620, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1015, in __get__
    _("(Record: %s, User: %s)") % (record, env.uid),
odoo.exceptions.MissingError: ('Record does not exist or has been deleted.\n(Record: ir.module.module(398,), User: 1)', None)

I think you can fix this by removing the related ir.model.data record.

Rad0van · 2020-08-18T18:37:25Z

It seems it is a CacheMiss Error. I have seen similar errors before but I really don't know how to solve this. Perhaps @hbrunn or @StefanRijnhart can help us here.

ERROR: TestDatabaseCleanup.test_database_cleanup_modules
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/api.py", line 745, in get
    value = self._data[field][record._ids[0]]
KeyError: 398
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1002, in __get__
    value = env.cache.get(record, self)
  File "/home/travis/odoo-13.0/odoo/api.py", line 751, in get
    raise CacheMiss(record, field)
odoo.exceptions.CacheMiss: ('ir.module.module(398,).views_by_module', None)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/travis/odoo-13.0/odoo/models.py", line 5796, in process
    recs.mapped(field.name)
  File "/home/travis/odoo-13.0/odoo/models.py", line 5210, in mapped
    recs = recs._mapped_func(operator.itemgetter(name))
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in _mapped_func
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5169, in <listcomp>
    vals = [func(rec) for rec in self]
  File "/home/travis/odoo-13.0/odoo/models.py", line 5620, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/home/travis/odoo-13.0/odoo/fields.py", line 1015, in __get__
    _("(Record: %s, User: %s)") % (record, env.uid),
odoo.exceptions.MissingError: ('Record does not exist or has been deleted.\n(Record: ir.module.module(398,), User: 1)', None)

I think you can fix this by removing the related ir.model.data record.

@CasVissers-360ERP could you please be more specific? What do you mean I remove that record? The thing is this error only happens in Travis. When I run tests on new clean instance of database in my local environment it doesn't happen. The second line from top in stacktrace is call to this: purge_modules = self.env["cleanup.purge.wizard.module"].create({}) So it attempts to create an instance of that class and fails on cache miss. As if the module/class wans't installed properly.

sergioeix · 2020-10-01T09:34:49Z

I tested https://github.com/Rad0van/server-tools/tree/13.0-mig-database_cleanup/database_cleanup on my side (oca modules and some store modules installed) and i have to say that works like a charm, and didnt noticed any issue, i wil do more test on other enviroments and let you know.
Hope my feedback will help in some way.
B/R

gustavovalverde · 2021-01-10T14:19:35Z

Hi, I was going to migrate this module, but I've seen 3 failed attempts (PR) already, 2 of them (including this one) being stuck on Travis with no specific reason.

Is it an issue on Travis? cc: @moylop260 @StefanRijnhart

StefanRijnhart · 2021-01-11T07:54:44Z

About the Travis error: this module test code is by necessity a bit tricky, performing actual commits on the database. In this case https://github.com/OCA/server-tools/pull/1885/files#diff-bcf55f79eb1510415055d7ad3c2e195e24a777c4a95b48a6701cba91f6776d74R172 it does so to delete the dummy module that is created in the test. SQL mutations are often the cause of MissingErrors as they cause consistencies between the SQL data and the Odoo cache.

That makes me think that maybe this issue is solved by calling the super's TearDown after the query, as the super clears the cache.

StefanRijnhart · 2021-01-11T07:56:17Z

@gustavovalverde Please go ahead porting this one to 14. Maybe you are able to fix the caching issue in your PR, and then the fix can be backported to this PR.

Rad0van · 2021-01-12T09:13:01Z

About the Travis error: this module test code is by necessity a bit tricky, performing actual commits on the database. In this case https://github.com/OCA/server-tools/pull/1885/files#diff-bcf55f79eb1510415055d7ad3c2e195e24a777c4a95b48a6701cba91f6776d74R172 it does so to delete the dummy module that is created in the test. SQL mutations are often the cause of MissingErrors as they cause consistencies between the SQL data and the Odoo cache.

That makes me think that maybe this issue is solved by calling the super's TearDown after the query, as the super clears the cache.

Stefan, it seems to work!

StefanRijnhart · 2021-01-12T14:55:59Z

Great to hear! About the code, it seems that you did not commit the prehook changes in a separate commit. Would it be alright for you to try and forge that in? See https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-13.0.

The way that formatting and actual changes are combined in a single commit makes it a little hard to review.

StefanRijnhart · 2021-01-12T14:57:39Z

+        for field in self.mapped("field_id"):
            model = self.env[field.model]
-            name = '%s_%s_index' % (model._table, field.name)
+            name = "{}_{}_index".format(model._table, field.name)


Please don't replace string substitution by format. See https://github.com/OCA/odoo-community.org/blob/master/website/Contribution/CONTRIBUTING.rst#33idioms

Was it actually pre-commit that did this?

Was it actually pre-commit that did this?

I guess so. I didn't do that knowingly.

I see it's pyupgrade from our pre-commit that does that, but only when it's safe so it's alright. I'm asking the lint-masters about it here: OCA/pylint-odoo#302 (comment)

Rad0van · 2021-01-12T15:00:24Z

Great to hear! About the code, it seems that you did not commit the prehook changes in a separate commit. Would it be alright for you to try and forge that in? See https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-13.0

I am always trying to follow that. Oh I see now. How can I "forge that in"? Not a clue on this.

avoid ''NoneType' object has no attribute 'exists'' error when purging models fix my change guewen.baconnier@camptocamp.com-20140203103254-v1mzu2uib047xb9h, wrong lines replaced...

raw SQL query (but never read afterwards). Workaround for lp:1277899 [FIX] Preserve dangling workflow table which is in use [RFR] Group models per table when detecting columns to purge to prevent problems with models sharing the same table [ADD] Allow purging of dangling data entries [FIX] Data purging now working [IMP] Docstrings [FIX] Label [FIX] Catch attempt to unlink field from nonexisting model [RFR] Flake8

[CHG] database_cleanup: move description to README.rst

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. 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/

Rad0van · 2021-01-12T16:11:55Z

OK, should be done. Messy way. But done :-)

StefanRijnhart · 2021-01-14T20:33:53Z

            )
            tables.add(model._table)
-        for table in tables:
+        for _table in tables:


Guessing you added the underscore because the variable is not changed? I'm thinking that is a bug and the table variable should be passed instead of model._table in the line below. Can you confirm @hbrunn?
As always, you can blame it on the reviewers (#736)

Guessing you added the underscore because the variable is not changed? I'm thinking that is a bug and the table variable should be passed instead of model._table in the line below. Can you confirm @hbrunn?
As always, you can blame it on the reviewers (#736)

Well to tell the truth I do not remember. But I guess the reason was the following line which reads:
self.env.cr.execute("analyze %s", (IdentifierAdapter(model._table),))
So without that change the table variable was never used.

Indeed, but the point is that _table is still not used, but model._table is used instead.

Oops, you're right. Now that's strange as original code did the same AFAICT. So this probably needs more investigation to get it right.

Yes, it was there from the original PR on. But logically, it only makes sense to use table there instead of model._table.

Yes, it was there from the original PR on. But logically, it only makes sense to use table there instead of model._table.

Sure, will do.

StefanRijnhart · 2021-01-14T21:30:01Z

                    "DELETE FROM ir_model WHERE id in %s", (tuple(self.models.ids),)
                )
            cr2.commit()
+            super(TestDatabaseCleanup, self).tearDown()


Trivial, but scoping the call to super under the cursor contextmanager makes less sense. Just unindent.

Trivial, but scoping the call to super under the cursor contextmanager makes less sense. Just unindent.

Stupid me. Will do.

bealdav

Other super call to check.
Thanks

bealdav · 2021-01-15T08:12:35Z

+                    continue
+            if this.model not in self.env:
+                this.unlink()
+        return super(IrModelData, self)._module_data_uninstall(modules_to_remove)


Suggested change

return super(IrModelData, self)._module_data_uninstall(modules_to_remove)

return super()._module_data_uninstall(modules_to_remove)

bealdav

LGTM

OCA-git-bot · 2021-01-25T15:48:55Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

StefanRijnhart

Thanks for the updates! (on the side: you responded to my comments with 'will do' but it's more helpful to comment after you have applied the requested changes so that the reviewer knows that the PR is ready for re-review)

StefanRijnhart · 2021-01-26T07:29:45Z

/ocabot merge nobump

OCA-git-bot · 2021-01-26T07:29:56Z

This PR looks fantastic, let's merge it!
Prepared branch 13.0-ocabot-merge-pr-1885-by-StefanRijnhart-bump-nobump, awaiting test results.

OCA-git-bot · 2021-01-26T07:44:40Z

Congratulations, your PR was merged at 0520ea7. Thanks a lot for contributing to OCA. ❤️

AaronHForgeFlow mentioned this pull request Aug 16, 2020

[13.0][MIG]database_cleanup #1823

Closed

Rad0van closed this Aug 16, 2020

Rad0van reopened this Aug 16, 2020

Rad0van closed this Aug 16, 2020

Rad0van reopened this Aug 16, 2020

Rad0van force-pushed the 13.0-mig-database_cleanup branch from 45d1871 to 2dc7753 Compare January 12, 2021 09:50

StephaneMangin mentioned this pull request Jan 12, 2021

[MIG] database_cleanup: Migration to 14.0 #1990

Closed

StefanRijnhart reviewed Jan 12, 2021

View reviewed changes

Rad0van force-pushed the 13.0-mig-database_cleanup branch from 2dc7753 to 2680094 Compare January 12, 2021 15:41

Stefan Rijnhart and others added 4 commits January 12, 2021 16:49

[ADD] Database cleanup module

79fe2a6

remove relations when purging models

3c6177a

avoid ''NoneType' object has no attribute 'exists'' error when purging models fix my change guewen.baconnier@camptocamp.com-20140203103254-v1mzu2uib047xb9h, wrong lines replaced...

[CHG] Migration to 8.0

93338d9

[CHG] database_cleanup: move description to README.rst

oca-travis and others added 8 commits January 12, 2021 16:49

[UPD] Update database_cleanup.pot

f77f262

database_cleanup 12.0.1.0.3

fdf2891

Update translation files

0cccc7e

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. 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/

[FIX] unlink cronjobs for the model before purging it

36aa490

[UPD] Update database_cleanup.pot

39decf1

database_cleanup 12.0.1.0.4

d8f0ab4

Update translation files

9c2b350

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. 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/

[IMP] database_cleanup: black, isort, prettier

352cb23

Rad0van force-pushed the 13.0-mig-database_cleanup branch from 2680094 to 3c4b4e9 Compare January 12, 2021 15:56

StefanRijnhart reviewed Jan 14, 2021

View reviewed changes

bealdav reviewed Jan 15, 2021

View reviewed changes

[MIG] database_cleanup: Migration to 13.0

04456f4

Rad0van force-pushed the 13.0-mig-database_cleanup branch from 9b0a300 to 04456f4 Compare January 25, 2021 13:55

hbrunn approved these changes Jan 25, 2021

View reviewed changes

Rad0van requested review from StefanRijnhart and bealdav January 25, 2021 15:27

bealdav approved these changes Jan 25, 2021

View reviewed changes

OCA-git-bot added approved ready to merge labels Jan 25, 2021

StefanRijnhart approved these changes Jan 26, 2021

View reviewed changes

OCA-git-bot added the bot is merging ⏳ label Jan 26, 2021

OCA-git-bot merged commit 5471c68 into OCA:13.0 Jan 26, 2021

OCA-git-bot added merged 🎉 and removed bot is merging ⏳ labels Jan 26, 2021

	return super(IrModelData, self)._module_data_uninstall(modules_to_remove)
	return super()._module_data_uninstall(modules_to_remove)

Uh oh!

Uh oh!

Conversation

Rad0van commented Aug 15, 2020

Uh oh!

Rad0van commented Aug 15, 2020

Uh oh!

AaronHForgeFlow commented Aug 16, 2020

Uh oh!

Rad0van commented Aug 16, 2020

Uh oh!

AaronHForgeFlow commented Aug 16, 2020

Uh oh!

Rad0van commented Aug 16, 2020

Uh oh!

Rad0van commented Aug 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AaronHForgeFlow commented Aug 16, 2020

Uh oh!

Rad0van commented Aug 16, 2020

Uh oh!

CasVissers-360ERP commented Aug 18, 2020

Uh oh!

Rad0van commented Aug 18, 2020

Uh oh!

sergioeix commented Oct 1, 2020

Uh oh!

gustavovalverde commented Jan 10, 2021

Uh oh!

StefanRijnhart commented Jan 11, 2021

Uh oh!

StefanRijnhart commented Jan 11, 2021

Uh oh!

Rad0van commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

StefanRijnhart commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Rad0van commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Rad0van commented Jan 12, 2021

Uh oh!

StefanRijnhart Jan 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bealdav left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bealdav left a comment

Choose a reason for hiding this comment

Uh oh!

OCA-git-bot commented Jan 25, 2021

Rad0van commented Aug 16, 2020 •

edited

Loading

Rad0van commented Jan 12, 2021 •

edited

Loading

StefanRijnhart commented Jan 12, 2021 •

edited

Loading

Rad0van commented Jan 12, 2021 •

edited

Loading

StefanRijnhart Jan 14, 2021 •

edited

Loading