[10.0][FIX] knowledge: migrations scripts

[MiquelRForgeFlow]

• Checked noupdate=1 records
• Included analysis work file
• Included migration scripts
• Updated documentation

[hbrunn]

for migrations out of openupgrade, you either should make the module depend on openupgradelib, which you probably don't want, or not use openupgrade. Just run the sql statement manually

[MiquelRForgeFlow]

true 😱

thanks

[pedrobaeza]

I don't agree. You don't need to depend on openupgradelib, as migrations are only triggered if there's a migration needed, not installing the module, so no issue if someone installs it directly on v9, and the only way to trigger this migration script is from OpenUpgrade.

[hbrunn]

...or from runnning a homebrew migration, or whatever else people do out there

[pedrobaeza]

But it's legitimate to ask for openupgradelib on that cases. I have already developed a dozen of migration scripts with this principle and they are already on OCA. It's not a closed library, so you have to know that for migrating OCA modules, you need to have OCA's openupgradelib, and if not, an error appears that blocks the upgrade, but you can install it, and continue migrating without any side effect.

[JordiBForgeFlow]

I agree with @pedrobaeza. It is an OCA tool, and helps developers to get the job done faster. Why restrict it to openupgrade migrations? We want to maximize adoption of these tools

[hbrunn]

I disagree with both of you in case what you propose is not to make the module depend on openupgradelib. If we include the dependency, then I can live with it

[pedrobaeza]

@hbrunn but again you don't need to depend on openupgradelib if you are going to install the module in a fresh 10.0 DB. You only need it when migrating from 9.0 to 10.0, so using OpenUpgrade for migrating or using any other custom migration, it's expected to have openupgradelib for this migration, and if not, I repeat again that the error is very clear, so you can recover easily from it.

[hbrunn]

you make a lot of assumptions on what people do. I'm not so convinced those are true.

[pedrobaeza]

Well, at the end, everything is an assumption when dealing with an heterogeneous system: if you are migrating from XML-RPC for example, the entry in depends manifest key doesn't matter also.

Does this really matter for the usual cases? Or do you have any migrating process in mind that we should take care of? If not, I prefer to stick to this assumption. If there's any, we can analyze to get to a common solution. But replacing a simple line for making a clear operation by a bunch of lines that can contain errors because they unfold a well known and tested method...

If the only solution for you is to add it as dependency, then go ahead. It's not a problem for me as we install in all our deployments openupgradelib. But as the usual flow for this module will be an installation o a fresh DB, I wouldn't want to force users to install openupgradelib because they won't really need it.

[JordiBForgeFlow]

@hbrunn but it not about what people do, but more what the tool offers. You can document in the README the assumptions/requirements to run a migration. Why let everyone be free to do whatever they want? Do you expect them to run odoo using mySQL, for instance? ;)

[pedrobaeza]

Example of proper installation documentation: https://github.com/OCA/delivery-carrier/pull/113/files#diff-4d43f50ed934cd213cc82350f76565aaR30

[hbrunn]

let's do some boring middle ground instead of a probably never ending discussion: Wrap the import in an exception handler, and assign some fake decorator to it in the except block that causes the call to be a noop. then i can't think of a scenario where this could go wrong

[hbrunn]

@jbeficent being "free to do whatever [I] want" is one of the more important reasons for me to be into open source ;-)

[hbrunn]

here's what i think we can use as blueprint:

try:
    from openupgradelib import openupgrade
except ImportError:
    openupgrade = lambda: None
    openupgrade.migrate = lambda *args, **kwargs:\
        lambda *args, **kwargs:\
            lambda cr, version=None: None

@openupgrade.migrate()
def migrate(cr, version=None):
    print 'with the handler above, I won\'t be run if there\'s no '\
        'openupgradelib, but nothing breaks'

if __name__ == '__main__':
    # without openupgradelib, nothing happens here, with it, it will fail
    # because we should have passed a cursor
    migrate(None, 'nothing')

[pedrobaeza]

But the problem with your blueprint is that this will silently don't make the migration for the module, which will be critical for version migrations (v8 > v9, v9 > v10), and it may crash later because a change that must be done in the migration scripts is not done (for example, a duplicate security group name). What's your problem with Odoo's update process in case of a migration between versions being interrupted until openupgradelib is installed?

[MiquelRForgeFlow]

Any conclusion here? Should I ping more people to have more opinions in the discussion?

[JordiBForgeFlow]

@hbrunn @pedrobaeza This PR is good, and we need to move forward. We have opted not to use openupgradelib this time. We copy-pasted the inside rename_xmlids. After all, openupgradelib is not yet being widely used in other OCA repos.

[pedrobaeza]

OK for this time. You know my preference for the general stuff.