Skip to content

Conversation

@cubells
Copy link
Member

@cubells cubells commented Aug 21, 2017

These changes are needed to upgrade connector module to 10.0

cc @Tecnativa

@cubells cubells changed the title [ADD] queue_job module: migration from 9.0 [ADD] [WIP] queue_job module: migration from 9.0 Aug 21, 2017
@OCA OCA deleted a comment from coveralls Aug 21, 2017
@cubells cubells changed the title [ADD] [WIP] queue_job module: migration from 9.0 [ADD] queue_job module: migration from 9.0 Aug 21, 2017
@pedrobaeza pedrobaeza merged commit 503e52f into OCA:10.0 Aug 24, 2017
@pedrobaeza pedrobaeza deleted the 10.0-mig_connector branch August 24, 2017 17:01
@lasley
Copy link
Contributor

lasley commented Aug 24, 2017

Curious - what's the rationale behind including the major version migrations here vs OpenUpgrade? If it's something fully supported by OCA, we migrate in the module?

@pedrobaeza
Copy link
Member

It's simple, this should be executed when this module gets upgraded. We can't include this in OpenUpgrade if we want to keep the modularity, or we will fill OpenUpgrade with a lot of if openupgrade.is_module_installed(... and the logic will be more complicated, as we must know where is the proper place to perform the upgrade.

@lasley
Copy link
Contributor

lasley commented Aug 24, 2017

Totally makes sense, thanks Pedro

@lmignon
Copy link
Contributor

lmignon commented Aug 25, 2017

@pedrobaeza @cubells @rafaelbn You must revert this merge! The queue_job addon doesn't depend on 'connector' and don't exist in 9.0.

@pedrobaeza
Copy link
Member

This is for 10.0, not for 9.0.

@guewen
Copy link
Member

guewen commented Aug 25, 2017

@pedrobaeza yes but how would that work? queue_job is a new addon in 10.0 so you'd install it and the migration won't be called. And if it is called you might not have the connector addon in your setup and you shouldn't be forced to use it because you are using queue_job.

@pedrobaeza
Copy link
Member

This will only be activated on migrations, not on regular installation, so there's no problem.

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.

6 participants