[SAAS-3] Migration script no more called on a fresh database

linked to opw-607346

Hi,

Why do you mark the bug as invalid? How can I distinguish first install from upgrade? Why do you remove a powerful functionality from something working very well?

The _init method on the model is called when the model is installed the first time but also each time it's upgraded. If I need to do a cost effective operation to initialize data/ migrate existing date, the first time a module is installed, and not each time the module is upgraded, I no more have a way this kink of operation. Moreover, the _init method is available only on one model and if I need to do some kink of 'initialization' once all models from the addons are installed, I no more have a way to do this kink of operation.

Do you have an other solution?

Hi Laurent,

As explained in the sent mail, you should use the init method. Using `migrate` for such use isn't a good idea and that's why it has been removed.

Why ? You create an index because your module makes usage of the related columns. It's relevant to add the index when you install the module with or without the migration scripts.

Beside this, having a call on migrate when installing force you the take care of that case in the migration scripts. Is it normal ? I don't think so.

So, I would check if the constraint is available in pg_indexes, and if not add the index.
See this message from the PostgreSQL mailing list for more information about it : http://<email address hidden>

Hi Anaël,

The init method is too limited. The scope of the init method is limited to
a Model in an addons. The scope of a migration script is for all the
addons. For exemple, in a init method we don't know if others models from
the addons are already installed.

The possibility to use a migration script when installing an addons offers
a lot more flexibility and features than the init method and I don't
understand why you've removed this possibility.

As written in a mail to the support, in some cases, before or after
installing an addons (but only on a first install) we need to run scripts
to migrate existing data in the database according to what is expected by
our newly installed addons. (for example, insert values based on queries
for the newly created columns in the database). These scripts can be very
heavy and should in no case be run a second time.

What do you propose as an alternative to cover such needs now that it is no
longer possible via the MigrationManager?
The _init method on the BaseModel is not an alternative since it's called
whenever the addons is installed AND upgraded and the method is scoped at
the 'Model' level not at the 'addons' level.

Is the change introduced in the revision https://bugs.launchpad.net/
openobject-server/+bug/1314680 really appropriate? If a migration script
should not be used to install an addons we add a test at the beginning of
the script:
    if not version:
        return

Regards,

lmi

On Wed, May 7, 2014 at 2:15 PM, Anaël Closson (openerp) <email address hidden>wrote:

> Hi Laurent,
>
> As explained in the sent mail, you should use the init method. Using
> `migrate` for such use isn't a good idea and that's why it has been
> removed.
>
> Why ? You create an index because your module makes usage of the related
> columns. It's relevant to add the index when you install the module with
> or without the migration scripts.
>
> Beside this, having a call on migrate when installing force you the take
> care of that case in the migration scripts. Is it normal ? I don't think
> so.
>
> So, I would check if the constraint is available in pg_indexes, and if not
> add the index.
> See this message from the PostgreSQL mailing list for more information
> about it :
> http://<email address hidden>
>
> --
> You received this bug notification because you are a member of Acsone
> OpenErp Team, which is subscribed to the bug report.
> https://bugs.launchpad.net/bugs/1314680
>
> Title:
> [SAAS-3] Migration script no more called on a fresh database
>
> Status in OpenERP Server:
> Invalid
>
> Bug description:
> In rev
> http://bazaar.launchpad.net/~openerp/openobject-server/saas-1/revision/4906,
> you have removed the call to the migration script on empty database or the
> first time a module is installed.
> The change has only been applied on saas-x and trunk branches but not
> backported to 7.0. This change is annoying since we no more have the
> possibility to use the migration machinery when installing a new module.
> Before the change, it was easy to distinguish in the migration script, a
> first installation of the module or a simple upgrade and therefore add ...

I totally agree with Laurent : this is a very bad regression from my point of view. The community modules need this feature to "migrate" existing data when a module is installed which e.g. computes the value of a new column from the values in existing record rows. This typically needs to be done only once at module install time, and it is a waste of ressource to require these data migration actions to be performed in init which means an extra call at each start of the instance (and maybe even at each start of a worker?), which could mean a query over several 100k of 1M of rows.

Please revert this change, which is useless and harming your users.

I'm also strongly in favor of keeping this behavior, please revert that change.

I also think that given arguments are enough to support the asking of not removing this feature. It's very simple, because it was already there. We are not asking to develop something new.

Regards.

It is **migration** scripts. not initialisation scripts.
They are intended to be one-shot modifications.

For initialisation, use noupdate data declared in __openerp__.py
I also remind you you can include sql files in __openerp__.py

OpenERP will automatically compute the value of the newly created stored function and relatedd fields. For others, the default value is used.

I taked a look at the migration scripts available on apps. Either they starts with "if not version: return", would have to, or, worst, are wrong as they recompute what must be related fields.
I even found one that create a plpgsql function!

This also error prone as every migration script would have to start with a check on version. C2C have already been bitten by this bug: https://bugs.launchpad.net/openerp-product-attributes/+bug/1259975

Hi

As Pedro says, we are not asking to develop something new...

I'm tired to try to explain why this feature is so useful. It's just counterproductive to remove this feature.

While the MigrationManager could be used to centralize the processes required to install or upgrade an addon, you give us an answer that it is better to disseminate these processes in different formats and approaches. (xml, sql, ...)

Regards,

lmi

Christophe, you are right about possible bad uses of this feature, but you can't also deny the possible advantages of another way of doing things. Then, people can make what crazy things they figure out with this feature, or have a bug like the one you mention, but they can also use it very cleverly to make some things that cost a lot to make in other ways. Remember that people still have cr.commit calls in code of their personal branches, but in OCA branches we try to control that level of code quality.

About the piece of code: "if not version: return", it's only a question of documentation for your part explaining that if version is None, it means a fresh install.

Regards.

On 05/08/2014 06:03 PM, Christophe Simonis (OpenERP) wrote:
> It is **migration** scripts. not initialisation scripts.
> They are intended to be one-shot modifications.
>
> For initialisation, use noupdate data declared in __openerp__.py
> I also remind you you can include sql files in __openerp__.py
Can you decide if they are run *before* or *after* the installation?
>
> OpenERP will automatically compute the value of the newly created stored
> function and relatedd fields. For others, the default value is used.

Are you aware that stored function fields will be computed one row at a
time, making the installation of a module nearly impossible when you
have hundreds of thousands of records?

In some cases, we *have* to help the ORM by precomputing the values of
stored function fields.

This is a good example:

https://code.launchpad.net/~acsone-openerp/account-financial-report/7.0-bug-1312732-lmi/+merge/217240

What are our possibilities?

1. Do nothing and let the stored function fields to their job:
impossible, due to the number of account move lines, it processes for
way too long
2. In init(): it was done in init() but each upgrade of the module
launch the query again. The query takes a lot of time.
3. Use a migration script that is run only at the installation of the
module, this was the better choice, until you removed it.

Also, sometimes, you have to prepare a database before the installation
of the module or run something after all is installed, and there is no
other possibilities than using migration scripts here.

>
> I taked a look at the migration scripts available on apps. Either they starts with "if not version: return", would have to, or, worst, are wrong as they recompute what must be related fields.
> I even found one that create a plpgsql function!
>
> This also error prone as every migration script would have to start with
> a check on version. C2C have already been bitten by this bug:
> https://bugs.launchpad.net/openerp-product-attributes/+bug/1259975
>
>
> ** Changed in: openobject-server
> Status: In Progress => Invalid
>

No, stored function fields are not recomputed one at a time.
In 7.0 they are recomputed 40 by 40 [0]. Yes, it may be low but that the default since Feb 2008 [1]. This value has been bump to 1000 in trunk.
One thing that may cause slowness was that, in case of m2o function fields, the name_get() was done id by id. This has been fixed last month in trunk [2]

About, your good example, that's effectively a good example how to NOT do a migration script.
Stored function fields are computed (in batch) in _auto_init(), before init() and also BEFORE migration post- scripts are executed.
Your module is not slow because of this query in init() but because your function field is badly written. You execute a query per id [3] instead of executing a single query with all the ids the ORM give you.

[0] http://bazaar.launchpad.net/~openerp/openobject-server/7.0/view/head:/openerp/osv/orm.py#L82
[1] http://bazaar.launchpad.net/~openerp/openobject-server/trunk/revision/702/bin/osv/orm.py
[2] http://bazaar.launchpad.net/~openerp/openobject-server/trunk/revision/5185/openerp/osv/fields.py
[3] http://bazaar.launchpad.net/~account-report-core-editor/account-financial-report/7.0/view/head:/account_financial_report_webkit/account_move_line.py#L58