OCA / server-tools

Tools for Odoo Administrators to improve some technical features on Odoo.
https://odoo-community.org/psc-teams/tools-30
GNU Affero General Public License v3.0
683 stars 1.47k forks source link

[RFC] upgrade_analysis: Detect removed SQL constraints #2221

Open pedrobaeza opened 2 years ago

pedrobaeza commented 2 years ago

For avoiding to not being aware of them.

Example:

https://github.com/odoo/odoo/blob/0eccd3973e056b4ac6fcd6754a8b6af2818aac4f/addons/survey/models/survey.py#L730

https://github.com/odoo/odoo/blob/1950b49b86deb44f33b6598a00bebc5b0fc42fa6/addons/survey/models/survey.py#L729-L731

It was fixed in https://github.com/OCA/OpenUpgrade/pull/2949, but detected later, and there are still some DBs around that have it being on upper versions.

Having this on the analysis file, will make easier to detect it and fix it on the first round.

@Tecnativa

legalsylvain commented 2 years ago

Hi. +1. At first sight, i see this feature better in database_cleanup. (Or maybe both because it's complementary) @StefanRijnhart any insight ?

Note : if you talk about a migration between 11 and 12, the feature will be in openupgrade / openupgrade_records module.

pedrobaeza commented 2 years ago

This is something that may break migration, so it should include it OpenUpgrade itself, and the idea is to annotate in the analysis file the missing constraints for taking actions in the migration scripts. It can be done only for newer versions, as previous ones are manually detected and fixed like the example I put.

legalsylvain commented 2 years ago

Naive question : I don't understand why it can break the migration.

How it can break the upgrade process ?

pedrobaeza commented 2 years ago

It's a good question. I have brought this because while migrating OCA database from 13 to 14, the error had arisen, as current code doesn't need to comply with it. If the constraint removal would be detected in time and removed, we wouldn't face this problem now. Even more, on regular use of the version, the constraint would be triggered incorrectly.

StefanRijnhart commented 2 years ago

It seems to me that the (manual or automatic) removal of the dropped constraints is required for a consistent datamigration. It could even be the case that a constraint still exists but with a different definition (that would be bad practice on the side of Odoo, but I would not rule it out). So it would be great to have such a change reflected in the analysis.

pedrobaeza commented 2 years ago

Other case I have just found that would be detected earlier having this analysis feature:

https://github.com/OCA/OpenUpgrade/commit/7a4090c71cb5197e1c72daf6e96d78ba6200b5dc