Skip to content

Conversation

@remytms
Copy link

@remytms remytms commented May 15, 2023

Analysis file is not generated by Upgrade Analysis.

I spotted theses changes:

models/res_config_settings.py:
del module_google_drive
del module_google_spreadsheet
del module_pad

These changes are done in a transient model so there is nothing to do (thanks @legalsylvain see below).

Theses fields trigger installation of modules that does not exists in 16.0 anymore. So I think it's ok to just do nothing during migration for base_setup.

models/res_config_settings.py:
del module_google_drive
del module_google_spreadsheet
del module_pad
@remytms
Copy link
Author

remytms commented May 15, 2023

This module is a dependency of #3779

@remytms
Copy link
Author

remytms commented May 15, 2023

I migrated a database with base_setup installed and with google_drive module installed. I get an error on the settings view. I will investigate to know if it's something that should be fixed in this module or by removing the old google_drive module.

@remytms
Copy link
Author

remytms commented May 15, 2023

The issue comes from the fact that the google_drive module was still installed.

@legalsylvain
Copy link
Contributor

legalsylvain commented May 15, 2023

Hi @remytms.

I spotted theses changes:

AFAIK, the changes are done in a transient model. changes in transient model are ignored by upgrade_analysis module, as there is nothing to do.

I migrated a database with base_setup installed and with google_drive module installed

I don't see the relation between base_setup and google_drive. base_setup depends on base + web in V15, and google_drive is not autoinstallable. in fact google_drive depends on base_setup and google_account.

The issue comes from the fact that the google_drive module was still installed.

I am not very comfortable with this use case. If I summary correctly. (please correct me if I'm wrong).

In that context,

  • if Openupgrade does nothing, the database is in an incorrect state in the end of the openupgrade V16 process that is not very desirable. (requieres manual uninstallation).
  • if OpenUpgrade uninstall the module, we will loose data present in google.drive.config model. (And it is not in the logic of openupgrade to delete data)

@pedrobaeza, @StefanRijnhart : how can openupgrade handle this use case elegantly?

I was wondering if writing _merged_modules = {"google_drive": "base"} in the 16.0 apriori.py file could be a solution.

@pedrobaeza
Copy link
Member

No, that's not possible. You are losing features, so no merge should be done. The module remains "to upgrade". You decide if uninstall it in a safe way, or migrate it to the new version, but nothing has to be done at OpenUgprade level.

@remytms
Copy link
Author

remytms commented May 15, 2023

AFAIK, the changes are done in a transient model. changes in transient model are ignored by upgrade_analysis module, as there is nothing to do.

Indeed. Thanks for spotting that.

I don't see the relation between base_setup and google_drive. base_setup depends on base + web in V15, and google_drive is not autoinstallable. in fact google_drive depends on base_setup and google_account.

The relation is that, on a 15.0 database, if you check the module_google_drive box, you end up with a database with google_drive module installed. So I wanned to check if such a database migrate correctly.

@remytms
Copy link
Author

remytms commented May 15, 2023

/ocabot migration base_setup

@OCA-git-bot
Copy link
Contributor

Sorry @remytms you are not allowed to mark the addon tobe migrated.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@legalsylvain
Copy link
Contributor

@remytms , limitation of the bot for the time being... only maintainers can call it.

/ocabot migration base_setup

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone May 15, 2023
@pedrobaeza
Copy link
Member

limitation of the bot for the time being...

It's a limitation put on purpose, for avoiding people hijacking other older migration PRs.

@pedrobaeza pedrobaeza merged commit c34b083 into OCA:16.0 May 15, 2023
@pedrobaeza
Copy link
Member

Next time please use [OU-ADD] as tag.

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.

4 participants