Skip to content

[IMP] util/records: skip replace if all ids are the same #253

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aj-fuentes
Copy link
Contributor

If all ids are the same and we are replacing in the same model we
currently cause an error. This is not necessary and could lead to false
errors if the list of mapping ids is correctly the same.

For example sometimes we transform a model A into model B by updating
metadata. Then we insert into B's table, and finally perform a replace
of references from A's table to B's table. This would fail if the ids
are the same. This case will happen with a high probability if there is
only one A record with id=1 since the single B record would get id=1 as
well.

Example logs after the patch:

2025-04-14 07:28:32,883 42811 WARNING test_16 odoo.upgrade.util.records: Replace references in model `res.country`, ignoring same-id mapping `{1: 1, 2: 2}`
2025-04-14 07:28:32,883 42811 WARNING test_16 odoo.upgrade.util.records: Nothing to replace in model `res.country`, ignoring empty mapping

@robodoo
Copy link
Contributor

robodoo commented Apr 14, 2025

Pull request status dashboard

@aj-fuentes aj-fuentes requested a review from KangOl April 14, 2025 14:45
Comment on lines 1472 to 1474
if not id_mapping:
if not id_mapping:
_logger.warning("Nothing to replace in model `%s`, ignoring empty mapping", model_src)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not id_mapping:
if not id_mapping:
_logger.warning("Nothing to replace in model `%s`, ignoring empty mapping", model_src)
if not id_mapping:
if not id_mapping:
if not id_mapping:
_logger.warning("Nothing to replace in model `%s`, ignoring empty mapping", model_src)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

rebase mishap :/

If all ids are the same and we are replacing in the same model we
currently cause an error. This is not necessary and could lead to false
errors if the list of mapping ids is correctly the same.

For example sometimes we transform a model A into model B by updating
metadata. Then we insert into B's table, and finally perform a replace
of references from A's table to B's table. This would fail if the ids
are the same. This case will happen with a high probability if there is
only one A record with id=1 since the single B record would get id=1 as
well.

Example logs after the patch:
```
2025-04-14 07:28:32,883 42811 WARNING test_16 odoo.upgrade.util.records: Replace references in model `res.country`, ignoring same-id mapping `{1: 1, 2: 2}`
2025-04-14 07:28:32,883 42811 WARNING test_16 odoo.upgrade.util.records: Nothing to replace in model `res.country`, ignoring empty mapping
```
@aj-fuentes aj-fuentes force-pushed the master-imp_avoid_error-afu branch from 283d018 to 133d859 Compare April 14, 2025 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants