[FIX] load_data: update to version 17.0

[royle-vietnam]

changed on 17.0
(odoo/odoo@5bf1207)

[pedrobaeza]

Thanks for this change. I'm wondering if we should instead convert the first argument to env_or_cr and delegate the responsibility of passing env or cr to the migration scripts, documenting it in the method that for 17+, the env is needed.

What do you think, @StefanRijnhart @legalsylvain @hbrunn ?

[legalsylvain]

I dont get it.

[pedrobaeza]

On v16-, you call the method load_data(cr, ...). On v17+, you call it load_data(env, ...).

[legalsylvain]

OK !

You propose to replace

def load_data(cr, module_name, filename, idref=None, mode="init"):

by

def load_data(env_or_cr, module_name, filename, idref=None, mode="init"):

Adding a comment to say that an cr is required until 16 and an env is required since 17, right ?

Personnaly, I prefer to keep the same signature def load_data(cr, module_name, filename, idref=None, mode="init"):

Then, add :

env_or_cr = api.Environment(cr, SUPERUSER_ID, {}) if version_info[0] >= 17 else cr

then simply replace tools.convert_xml_import(cr, module_name, fp, idref, mode=mode)
by tools.convert_xml_import(env_or_cr, module_name, fp, idref, mode=mode)

But both works, so it's maybe a matter of taste. I think that it can maybe generate more errors to change the type of the argument, depending of the version of the module that call the function.

[royle-vietnam]

OK !

You propose to replace

def load_data(cr, module_name, filename, idref=None, mode="init"):

by

def load_data(env_or_cr, module_name, filename, idref=None, mode="init"):

Adding a comment to say that an cr is required until 16 and an env is required since 17, right ?

Personnaly, I prefer to keep the same signature def load_data(cr, module_name, filename, idref=None, mode="init"):

Then, add :

env_or_cr = api.Environment(cr, SUPERUSER_ID, {}) if version_info[0] >= 17 else cr

then simply replace tools.convert_xml_import(cr, module_name, fp, idref, mode=mode) by tools.convert_xml_import(env_or_cr, module_name, fp, idref, mode=mode)

But both works, so it's maybe a matter of taste. I think that it can maybe generate more errors to change the type of the argument, depending of the version of the module that call the function.

OK !

You propose to replace

def load_data(cr, module_name, filename, idref=None, mode="init"):

by

def load_data(env_or_cr, module_name, filename, idref=None, mode="init"):

Adding a comment to say that an cr is required until 16 and an env is required since 17, right ?

Personnaly, I prefer to keep the same signature def load_data(cr, module_name, filename, idref=None, mode="init"):

Then, add :

env_or_cr = api.Environment(cr, SUPERUSER_ID, {}) if version_info[0] >= 17 else cr

then simply replace tools.convert_xml_import(cr, module_name, fp, idref, mode=mode) by tools.convert_xml_import(env_or_cr, module_name, fp, idref, mode=mode)

But both works, so it's maybe a matter of taste. I think that it can maybe generate more errors to change the type of the argument, depending of the version of the module that call the function.

@legalsylvain Thank you for your solution. I find it very interesting and low risk. I have fixed it according to your solution.

[pedrobaeza]

Why I was talking to pass env is for the same reason as stated in the Odoo commit: to re-use the environment and avoid to regenerate it each time, and to share the context. Don't you think it's worth?

[royle-vietnam]

Why I was talking to pass env is for the same reason as stated in the Odoo commit: to re-use the environment and avoid to regenerate it each time, and to share the context. Don't you think it's worth?

@pedrobaeza I have updated it according to your suggestion, please review it

[pedrobaeza]

This one hasn't been changed, isn't it?

[royle-vietnam]

Has been changed at 5bf1207c8cb7575fba4501aff4932c2cc952bf9f

[StefanRijnhart]

yaml_import was removed from Odoo after 11 anyway.

[pedrobaeza]

I know, but right now, the code will fail in 11-, as cr has no value for that case. Assign previously cr = env_or_cr, or directly put here env_or_cr.

[royle-vietnam]

you are right, I added more at line 310

[pedrobaeza]

Thanks for the patience attending our comments.