[RFC] OCA modules migration scripts host

[pedrobaeza]

Due to the new design of OpenUpgrade in v14, we can host all the migration scripts here (Odoo core modules and OCA modules) in this repository, as they will be used the same as for the core modules. Let's balance the pros and cons:

This is summarizing whole thread

Pros of putting everything together in OCA/OpenUpgrade repo

• [1] Everything together for better discoverability.
• [2] Avoid noise on the business logic repository.
• [3] We can test such migration scripts with a centralized strategy and CI (something that we don't have currently and it's impossible to be done if not centralized).
• [4] On the same manner, we don't hammer other repos CIs testing the business logic for not testing at all a PR adding migration scripts.

Cons of putting everything together in OCA/OpenUpgrade repo:

• [1] The scripts can't be used by other upgrade mechanisms, like the enterprise one. I think this is not totally true (but I can't confirm for sure), as enterprise process will upgrade core modules, and then you can perform another update, and you are able to indicate this repo as migration scripts source, and only not yet updated modules will be run.

  I also don't think this is necessarily a cons, as some migration scripts are very coupled to the design decisions done on OpenUpgrade, so they will miserably fail when used with other migration process. For example, on v13, we put on account.move the field old_invoice_id for pointing to the legacy account.invoice record that links to the move. The rest of the modules that added fields on account.invoice and now are moved to account.move, contains migration scripts based on this assumption. You then decide if using this as migration path or not for your migration process.

• [2] This repository will get more dirty. True, but we can organize folders this way:

  \openupgrade_framework
  \openupgrade_scripts
  \oca_scripts

  and host in the last one the migration for OCA modules, and thus we split conveniently the number of contained folders.

• [3] When migrating a module to v14, you may want to add migration scripts at the same time, and with this structure, you are forced to do 2 PRs, one in the module repo, and one here. This one can't be avoided, but for me it's not a great trade-off.

• [4] Pointed by @JordiBForgeFlow: The closer the migration code is to where the actual code is, the better. Answer by @pedrobaeza: We have such isolation though on Odoo core/OpenUpgrade main migration scripts now on v14.

• [5] Pointed by @JordiBForgeFlow: migration scripts between major versions OCA modules are not so different from migration scripts of minor ones. Answer by @pedrobaeza: I think minor versions migration scripts are very rare and specific, while major versions scripts are common and very tied to OpenUpgrade itself, so not too much in common.

• [6] Inspired by @rvalyi: merges must be done by OpenUpgrade PSCs, not repo PSCs. Answer by @pedrobaeza: This can be workarounded with a delegate system like Odoo has in their repo, writing \ocabot delegate @OCA/accounting-maintainers or similar for allowing to merge to that people. This needs to be developed though.

As you can see in my rationale, I clearly see that including them here is a better approach, but maybe I'm missing some cons or PoV, so let's start the debate!

[JordiBForgeFlow]

In my opinion the closer the migration code is to where the actual code is, the better. Also migration scripts between major versions OCA modules are not so different from migration scripts of minor ones

[pedrobaeza]

Well, the same can apply then to Odoo core modules and OpenUpgrade migration scripts, and in v14 we have isolate both in the refactoring. Anyways, this opinion is welcomed and I'll add it to the cons.

[rvalyi]

In my opinion the closer the migration code is to where the actual code is, the better. Also migration scripts between major versions OCA modules are not so different from migration scripts of minor ones

Specially, we have to consider the design changes inside the same serie. Usually we have an atomtic PR with both the design change and its migration script, all goes inside the same merge once aproved together.

If we split the business logic and the OpenUpgrade script instead, it may be harder to guarantee a smooth user experience: for instance the module change 1st, break everyone installation and the OpenUpgrade script makes it only a few days later. You may also not checkout the 2 repos when you should...
And even if instead you try to ensure the migration enters the OpenUpgrade repo 1st, you may have another design change later due to the review process that will break it.

Also consider the discrepencies in the review process, the fact the number of OpenUpgrade PSC cannot scale to the diversity of the other modules repos PSCs... OpenUpdgrade PSC will likely have no clue what module change in localization X means, hence the review process may lag or instead be too permissive if localization guys start to approve generic OpenUpgrade wizardery they don't master...

[pedrobaeza]

Specially, we have to consider the design changes inside the same serie. Usually we have an atomtic PR with both the design change and its migration script, all goes inside the same merge once aproved together.

It's not the same thing. For changes inside same version, migration scripts will still be in the same repository, as they belong to them. This is only for strictly major version changes (from 13.0 to 14.0 for example).

If we split the business logic and the OpenUpgrade script instead, it may be harder to guarantee a smooth user experience: for instance the module change 1st, break everyone installation and the OpenUpgrade script makes it only a few days later. You may also not checkout the 2 repos when you should...
And even if instead you try to ensure the migration enters the OpenUpgrade repo 1st, you may have another design change later due to the review process that will break it.

Again this is a problem for changes inside same version. Even for changes across versions, when a module is migrated to a new version, it can delay a lot of time until migration scripts for previous version are proposed.

Also consider the discrepencies in the review process, the fact the number of OpenUpgrade PSC cannot scale to the diversity of the other modules repos PSCs... OpenUpdgrade PSC will likely have no clue what module change in localization X means, hence the review process may lag or instead be too permissive if localization guys start to approve generic OpenUpgrade wizardery they don't master...

This fact exists more or less doing it on the other OCA repository: you can be a PSC and don't know anything about a specific module to validate the migration scripts. Anyways, there's another cons that I'm annotating (with a possible workaround): merges must be done by OpenUpgrade PSCs, not repo PSCs.

[legalsylvain]

Hi @pedrobaeza. Thanks for your RFC, and for centralizing analysis.
I took the liberty of editing your text to add numbers, so that I could simply refer to it.

[legalsylvain]

Regarding the pro-arguments

• I am sensitive to the argument n°1. (better discoverability)
• I don't find the argument n°2 very important. Migration scripts are quite rare and are not generating a lot of noices, (IMHO)
• Regarding argument n°3 and n°4 (CI) : having a CI for migrations of OCA modules does not seem possible at this time, and not desirable, IMHO :
  I mean, migrations script of Odoo modules can be testable because once released, the modules are very stable (module of version N-1 and module of version N) : No changes in data model, very fiew breaking changes. It seems very rare that a change in a stable release requires to update a migration script.
  In reverse, in OCA, migrations script can work, and then be broken. Specially, if a module is not present in all the revision.

For exemple, for the web module web_dashboard_tile

• it is available for 7.0, 8.0, 9.0 and 10.0.
• it is not available for 11.0
• I worked on a migration for 12.0 here, including migration script, to improve the module.

Point A : how the test environment will know how to test that script ? based on the last version available ? (the 10.0)
Point B :
If my 12.0 PR is merged, and then backported in 11.0 it will break the migration of the module in version 12.0
And if the migrations script are done in the openupgrade repository (*), it will make red the Openupgrade repository, preventing maintainers to add new migrations once the migrations is fixed. But we are a very few maintainers on OpenUpgrade, and adding such extra work is not a gift to the team.
That point is quite important, as many times, modules are not available in all versions. It's a reality. (**) (I'm not saying that working only on even-numbered versions is a good or bad idea, that's another subject ;-)... Please, @rvalyi, don't...;-)

Regarding the cons-arguments

• Point 1. no idea regarding compatibility with enterprise migration. Considering this argument as invalid as long as developers using the Enterprise Migration have not spoken out on this subject.
• Point 2 : I don't think that setting all the migrations scripts in a same repository is less or more dirty. matter of taste.
• Point 4 :
  Pedro said : Well, the same can apply then to Odoo core modules and OpenUpgrade migration scripts, and in v14 we have isolate both in the refactoring
  We are not working on migration of Odoo modules and OCA modules in the same way. We don't develop (and makes changes on) odoo modules. We just discover changes in each releases (generally during the keynote) and have to develop migration scripts. Isolate migration scripts is not a problem for me in that context. whereas when we port OCA modules, we are free to do as we want, to change models or viewss / add or remove dependencies, so it makes sense to work in the same place in that context.
• Point 3, 5, 6. That points are important to me.

(*) : it is theoretical, because the version 12 of openupgrade is on the old design. But it's just an exemple.
(**) up to date OCA modules per revision :

[jcdrubay]

I would be in favor of not changing (keep migration scripts inside the OCA modules repos) for the following reasons:

• [a] From a technical point of view, the migration scripts for minor and major upgrade are the same. So I don't think that we should store them in different places.

• [b] Training of the possibilities of migration scripts as an introduction to OpenUpgrade. I believe that most of Odoo developers are unfamiliar with openupgradelib scripts because the use cases when needed are rare. Therefore, including the migration scripts for major version directly in the "OCA module" repos, we are promoting migration scripts and therefore facilitating the on-boarding of future OpenUpgrade developers.

• [c] the number of Issues / Pull request might expend fast and become difficult to control. I think it is reasonable to give the responsibility of the migration scripts to each repo maintainer. Repo maintainers will be more naturally involved (notifications, checking opened issues/PR on their repo) if migration scripts are in their repo than if they need to go check for issues assigned to them in the middle of all OpenUpgrade issues/PR from all OCA-modules-repo.

[pedrobaeza]

@legalsylvain don't understimate having the CI for OCA modules migration scripts. We are not talking about creating tests for them, but at least being run for not having an invalid SQL expression or data integrity in a real flow in a migration from previous version to current one. This test is very difficult to be done by scripts developers, as the needed playground for such thing is very huge. My idea on the CI workflow is to see subfolders (aka modules names) of the new folder oca_scripts, install them with the previous version, and then perform the migration. All the pieces are in place for that, as current CI does everything except the OCA modules install one. Such code of the CI can discover the last available version.

[rvalyi]

Something I was thinking about: may be we can find a way, for each repo to elect the modules wich are both the most mature and the most popular (warning announce you do all accounting for country X can drive a lot of hype, it doesn't necessarily translate to being a mature module, IMHO these modules should be both popular and mature/serious). For instance we can ask each PSC to give a shortlist of 10% of most mature+popular modules of their repo and start from there.

Then for these popular and mature modules, eventually, we also get their major serie migration tested in OCA/OpenUpgrade as @pedrobaeza suggested. But we would still get the scripts maintained in the modules repos. I also want to emphasis migrations between series and minor are not so different as some said, they are usually done by the same people and often a major change is in fact done inside the same serie. Specially because just the migration might be a lot of work so design changes are often postponed months later after the bare serie migration.

Random examples of both mature and popular modules I can think about:
web_responsive, account_financial_report, account_payment_order, account_invoice_date_due, helpdesk, stock_available,stock_secondary_unit, purchase_request, field_service, agreement, contract...

That would both serve as offering a stronger foundation basis for everybody, without creating a new monster that wouldn't scale and just make OpenUpgrade a mess IMHO.

[pedrobaeza]

@rvalyi usually the modules that you mention don't require migration scripts, and the list can be subjective, so I don't think doing some of them here and others inline would help, so I would say that is an all or nothing question, and for now it seems all of you prefer to leave it as is. If nothing changes in the coming days, I just close this and continue the same way.

[StefanRijnhart]

Agree with everything that @jcdrubay says in #2661 (comment). I would much prefer to keep the migration files together with their module code.

[SimoRubi]

I'm also in favor of keeping migration scripts of a module close to its code (i.e. inside the module itself).

About having migration scripts in OpenUpgrade:

• [1] Everything together for better discoverability.

  That is about taste, in my opinion everything together would be the migration script together with the module's code

• [2] Avoid noise on the business logic repository.

  As above, in my opinion that is not noise: migration scripts can be an important part of migration procedure for a module.
  They are also mentioned in some of OCA migration guides:

  [...] you need to add migration script [...]

  (ref. https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-13.0)

• [3] and [4] (about CIs)

  In my opinion CIs checking migration scripts should instead be added in each repo, in this way both major and minor version migration scripts will be verified by CI.
  I understand this is no easy task, but just seems reasonable to do so

For changes inside same version, migration scripts will still be in the same repository, as they belong to them. This is only for strictly major version changes (from 13.0 to 14.0 for example).

Managing migration scripts differently for major and minor version can be confusing for the developer, because they are basically the same thing.

[pedrobaeza]

OK, then I think it's not worth to keep this discussion. Let's close the RFC and keep as we are. Thanks all for your opinions.

[rvalyi]

@rvalyi usually the modules that you mention don't require migration scripts, and the list can be subjective, so I don't think doing some of them here and others inline would help, so I would say that is an all or nothing question, and for now it seems all of you prefer to leave it as is. If nothing changes in the coming days, I just close this and continue the same way.

Hello @pedrobaeza there is also an other issue with the "test all OCA module migration in OpenUpgrade" approach you suggested when you say "all or nothing". Bare in mind that it's impossible to even just install all modules together. It's already hard to get all modules on a single repo install together and get their test all passing... So if you would like to test these modules migration, I think you have to go for a shortlist no matter what, otherwise be prepared for adding extra 100+ Travis runs per OCA/Openupgrade test run (like one pass per repo). And again this raises a scalability issue... Again I praise the intention, I just question the way to do it.

[pedrobaeza]

You don't need to install everything. Just the modules that have migration scripts here (a few). Anyways, no need to discuss more, as most people don't want this, so let's focus in other things (like v14 migration scripts 😉)