Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] 9.0 base #404

Merged
merged 7 commits into from
Feb 11, 2016
Merged

[WIP] 9.0 base #404

merged 7 commits into from
Feb 11, 2016

Conversation

coleste
Copy link

@coleste coleste commented Nov 24, 2015

WIP for base

@coleste
Copy link
Author

coleste commented Nov 24, 2015

I think there are boostrap issues. preload_registries() tries to "select model, transient from ir_model where state='manual'".

I added it manually and re-ran the open-server call, and most of the new columns in ir.model.fields have the same issue.

@coleste
Copy link
Author

coleste commented Nov 24, 2015

Also, migrate.py did not work without bzr installed. I moved bzrlib.plugin.load_plugins() where it seemed right.

@pedrobaeza
Copy link
Member

You should make a PR for the bzr issue, but thanks for noticing that. About Bootstrap mention, why do you think that's the problem? OpenUpgrade doesn't need any web interface for working.

@coleste coleste mentioned this pull request Nov 24, 2015
@coleste
Copy link
Author

coleste commented Nov 24, 2015

No, sorry. I meant bootstrapping Odoo to execute the pre/post-migration scripts.

"select model, transient from ir_model" fails because transient is not in the migrated database.

@pedrobaeza
Copy link
Member

OK, you mean "patching" the server before starting the normal migration wheel. Yeah, we should see in what that impact and precreate and calculate its value before. But are you sure that can't be done in pre-migration base script?

@coleste
Copy link
Author

coleste commented Nov 24, 2015

I did put some print '****' in the pre-migrate.py, but nothing outputs. I guess the error happens before it gets there. Here's the migration.log:

2015-11-24 15:58:51,969 36494 INFO ? openerp: OpenERP version 9.0
2015-11-24 15:58:51,969 36494 INFO ? openerp: addons paths: ['/Users/stephane/Library/Application Support/Odoo/addons/9.0', u'/var/tmp/openupgrade/9.0/server/openerp/addons', u'/var/tmp/openupgrade/9.0/addons', '/private/var/tmp/openupgrade/9.0/server/openerp/addons']
2015-11-24 15:58:51,969 36494 INFO ? openerp: database: openerp@localhost:default
2015-11-24 15:58:52,196 36494 INFO v8migrate_migrated openerp.modules.loading: loading 1 modules...
2015-11-24 15:58:52,205 36494 INFO v8migrate_migrated openerp.sql_db: Programming error: column "transient" does not exist
LINE 1: select model, transient from ir_model where state='manual'
                      ^
, in query select model, transient from ir_model where state=%s
2015-11-24 15:58:52,205 36494 CRITICAL v8migrate_migrated openerp.service.server: Failed to initialize database `v8migrate_migrated`.
Traceback (most recent call last):
  File "/private/var/tmp/openupgrade/9.0/server/openerp/service/server.py", line 885, in preload_registries
    registry = RegistryManager.new(dbname, update_module=update_module)
  File "/private/var/tmp/openupgrade/9.0/server/openerp/modules/registry.py", line 385, in new
    openerp.modules.load_modules(registry._db, force_demo, status, update_module)
  File "/private/var/tmp/openupgrade/9.0/server/openerp/modules/loading.py", line 311, in load_modules
    loaded_modules, processed_modules = load_module_graph(cr, graph, status, perform_checks=update_module, report=report, upg_registry=upg_registry)
  File "/private/var/tmp/openupgrade/9.0/server/openerp/modules/loading.py", line 150, in load_module_graph
    registry.setup_models(cr, partial=True)
  File "/private/var/tmp/openupgrade/9.0/server/openerp/modules/registry.py", line 185, in setup_models
    cr.execute('select model, transient from ir_model where state=%s', ('manual',))
  File "/private/var/tmp/openupgrade/9.0/server/openerp/sql_db.py", line 139, in wrapper
    return f(self, *args, **kwargs)
  File "/private/var/tmp/openupgrade/9.0/server/openerp/sql_db.py", line 215, in execute
    res = self._obj.execute(query, params)
ProgrammingError: column "transient" does not exist
LINE 1: select model, transient from ir_model where state='manual'
                      ^

2015-11-24 15:58:52,206 36494 INFO v8migrate_migrated openerp.service.server: Initiating shutdown
2015-11-24 15:58:52,206 36494 INFO v8migrate_migrated openerp.service.server: Hit CTRL-C again or send a second signal to force the shutdown.

@pedrobaeza
Copy link
Member

Well, that seems then. @StefanRijnhart, did you faced something similar in the past?

@hbrunn
Copy link
Member

hbrunn commented Nov 24, 2015

@coleste what is your commandline to start the server?

@coleste
Copy link
Author

coleste commented Nov 24, 2015

The server is started by migrate.py, that's what I used :

/var/tmp/openupgrade/9.0/server/openerp-server --update=all --database=v8migrate_migrated --config=/var/tmp/openupgrade/9.0/server.cfg --stop-after-init --no-xmlrpc

@hbrunn
Copy link
Member

hbrunn commented Nov 24, 2015

that's a perfectly good commandline, so unfortunately you'll have to dig deeper. Put a breakpoint at https://github.com/OCA/OpenUpgrade/blob/8.0/openerp/modules/loading.py#L169 and see what happens

@coleste
Copy link
Author

coleste commented Nov 24, 2015

It hits the breakpoint I put in exception handling of sql_db.py:217, never gets to your BP.

@StefanRijnhart
Copy link
Member

'transient' is not a column in 8.0, so we will need to intervene earlier on in the bootstrapping of the server and the upgrade mechanism. By the looks of it, manual models did not have a transient option at all before 9.0, so we can just create the column dynamically at this point (in registry.py) or wrap the statement in a condition that checks if the column exists. @coleste do you feel up to doing that? Propose it in an atomic PR and we'll review it quickly.

@coleste
Copy link
Author

coleste commented Nov 24, 2015

We'll have the same problem with ir.model.fields and ir.model.constraint, I had to hack 10 more columns into the DB to get further. And then there was a problem with select_level from ir.model.fields. Here are the bits from openupgrade_analysis.txt:

base / ir.model / transient (boolean) : NEW
base / ir.model.constraint / definition (char) : NEW
base / ir.model.fields / column1 (char) : NEW
base / ir.model.fields / column2 (char) : NEW
base / ir.model.fields / compute (text) : NEW
base / ir.model.fields / copy (boolean) : NEW
base / ir.model.fields / depends (char) : NEW
base / ir.model.fields / help (text) : NEW
base / ir.model.fields / index (boolean) : NEW
base / ir.model.fields / related (char) : NEW
base / ir.model.fields / relation_table (char) : NEW
base / ir.model.fields / select_level (selection) : DEL required: required, selection_keys: ['0', '1', '2'], req_default: 0

I can add a safety check in setup_models() to execute the SQL patch, I think it has to be in that Odoo file.

@hbrunn
Copy link
Member

hbrunn commented Nov 24, 2015

the differences from base.sql are

diff --git a/openerp/addons/base/base.sql b/openerp/addons/base/base.sql
index 9b42bac..a994cc9 100644
--- a/openerp/addons/base/base.sql
+++ b/openerp/addons/base/base.sql
@@ -19,6 +19,7 @@ CREATE TABLE ir_model (
   name varchar,
   state varchar,
   info text,
+  transient boolean,
   primary key(id)
 );

@@ -27,14 +28,23 @@ CREATE TABLE ir_model_fields (
   model varchar NOT NULL,
   model_id integer references ir_model on delete cascade,
   name varchar NOT NULL,
-  relation varchar,
-  select_level varchar,
+  state varchar default 'base',
   field_description varchar,
+  help varchar,
   ttype varchar,
-  state varchar default 'base',
+  relation varchar,
   relation_field varchar,
+  index boolean,
+  copy boolean,
+  related varchar,
+  readonly boolean default False,
+  required boolean default False,
+  selectable boolean default False,
   translate boolean default False,
-  serialization_field_id integer references ir_model_fields on delete cascade, 
+  serialization_field_id integer references ir_model_fields on delete cascade,
+  relation_table varchar,
+  column1 varchar,
+  column2 varchar,
   primary key(id)
 );

@@ -140,6 +150,7 @@ CREATE TABLE ir_model_constraint (
     module integer NOT NULL references ir_module_module on delete restrict,
     model integer NOT NULL references ir_model on delete restrict,
     type character varying(1) NOT NULL,
+    definition varchar,
     name varchar NOT NULL,
     primary key(id)
 );
@@ -185,11 +196,11 @@ insert into res_currency (id, name) VALUES (1, 'EUR');
 insert into ir_model_data (name, module, model, noupdate, res_id) VALUES ('EUR', 'base', 'res.currency', true, 1);
 select setval('res_currency_id_seq', 2);

-insert into res_company (id, name, partner_id, currency_id) VALUES (1, 'Your Company', 1, 1);
+insert into res_company (id, name, partner_id, currency_id) VALUES (1, 'My Company', 1, 1);
 insert into ir_model_data (name, module, model, noupdate, res_id) VALUES ('main_company', 'base', 'res.company', true, 1);
 select setval('res_company_id_seq', 2);

-insert into res_partner (id, name, company_id) VALUES (1, 'Your Company', 1);
+insert into res_partner (id, name, company_id) VALUES (1, 'My Company', 1);
 insert into ir_model_data (name, module, model, noupdate, res_id) VALUES ('main_partner', 'base', 'res.partner', true, 1);
 select setval('res_partner_id_seq', 2);

which is pretty much in line with what you did. I think I'd make the changes in https://github.com/OCA/OpenUpgrade/blob/9.0/openerp/modules/registry.py#L67

@hbrunn
Copy link
Member

hbrunn commented Nov 24, 2015

this should be its own commits as in 3426b63 - this makes it simpler to reapply in the future

@dreispt dreispt changed the title 9.0 base [WIP] 9.0 base Nov 25, 2015
@coleste
Copy link
Author

coleste commented Nov 25, 2015

With the 2 fixes (), I get to Element '' cannot be located in parent view, which means I'm back to fixing the migration itself.

@coleste
Copy link
Author

coleste commented Nov 27, 2015

Ok, inherit_id is not cleared by default, I had to modify report.layout to fix the migration.

This problem will occur again whenever a template inheritance is removed.

@coleste
Copy link
Author

coleste commented Nov 27, 2015

This should be done as soon as #405 and #407 are accepted, unless I missed something on the xml side.

@coleste coleste force-pushed the 9.0-base branch 7 times, most recently from f77a34e to 4d46bad Compare November 30, 2015 22:21
@coleste
Copy link
Author

coleste commented Dec 8, 2015

@pedrobaeza thank you. I also used logged_query() and migrated "Your Company".

Second stage database comparison returns this:

---Fields in module 'general'---
# 1093 fields matched,
# Direct match: 1077
# Found in other module: 0
# Found with different name: 0
# Found with different type: 0
# In obsolete models: 16
# New columns: 15
# Not matched: 0
obsolete model bus.bus
obsolete model bus.presence
---XML records in module 'general'---
ERROR: module not in list of installed modules:
---Fields in module 'bus'---
---XML records in module 'bus'---
DEL ir.model.access: bus.access_bus_bus
DEL ir.model.access: bus.access_bus_presence
DEL ir.model.access: bus.access_bus_presence_portal
DEL ir.ui.view: bus.assets_backend

@pedrobaeza
Copy link
Member

But I don't understand why are you running database comparison. That phase is already done. You should work on the migration of the base module itself.

@coleste
Copy link
Author

coleste commented Dec 8, 2015

@pedrobaeza That's the second pass, after the migration is done. Since it only talks about bus, it looks like I did not miss anything, no?

@pedrobaeza
Copy link
Member

Ah, OK, it seems correct then, but I have never checked the correctness of a migration script running again the comparison.

@coleste
Copy link
Author

coleste commented Dec 10, 2015

@hbrunn @StefanRijnhart I also found 'share' was removed in 9.0 when I tried further tests with 'mail' installed. This causes error because migrate cannot find the model 'share.wizard.result.line' et al.

It makes me wonder if you have to have the module present to uninstall it. I've hacked it back in but I wonder if there's a preferred solution.

@legalsylvain legalsylvain added this to the 9.0 milestone Dec 11, 2015
@mmalorni
Copy link

mmalorni commented Jan 5, 2016

@hbrunn @StefanRijnhart @pedrobaeza
#407 is merged as well.

Only remains to delete the modules that are gone in V9.
Is the following code the right way to do it or is there another way since it doesn't seem to work. (@coleste took the code from the port to V7)

remove_obsolete_modules(cr, ('im_chat', 'share', 'web_gantt', 'web_graph', 'web_tests'))

def remove_obsolete_modules(cr, modules):
openupgrade.logged_query(cr, """
update ir_module_module set state='to remove'
where name in %s
and state in ('installed', 'to install', 'to upgrade')
""" % (modules,))

@hbrunn
Copy link
Member

hbrunn commented Jan 5, 2016

better call module_uninstall (https://github.com/OCA/OpenUpgrade/blob/9.0/openerp/addons/base/module/module.py#L465) and then unlink

@coleste
Copy link
Author

coleste commented Jan 8, 2016

@hbrunn yes it's cleaner this way thank you. For the share module, I uninstall it in its own post-migration since it needs the python models to uninstall. I don't think there's a cleaner way to do it.

@StefanRijnhart
Copy link
Member

Thank you for your work so far, it's looking good!

I am not sure about re-adding the whole share module. Could you at least try with an empty module (no module files, views, static directory), just an openerp.py, an empty init.py and the migration script?

@StefanRijnhart
Copy link
Member

In fact, adding the share module makes the diff here on github cut out the interesting bits...

About ir.attachment's new checksum field, would you agree to calculate it for the attachments in the database? Otherwise it will be calculated on the fly again and again when requesting it from the web interface: https://github.com/odoo/odoo/blob/9.0/openerp/addons/base/ir/ir_http.py#L119

@StefanRijnhart
Copy link
Member

ir.filters' active field: you should set a default of True. The ORM only does this for you for new required fields.

@StefanRijnhart
Copy link
Member

New 'transient' column on ir.model: we will probably want to handle this in a deferred step, or in a modification of the module loading code. You don't need to worry about it now, we could add this as a TODO on the project.

@coleste
Copy link
Author

coleste commented Jan 14, 2016

I truncated share to its bare minimum, the models could be defined elsewhere but we need them because ir_model_constraint._module_data_uninstall() accesses model_obj._table.

@coleste
Copy link
Author

coleste commented Jan 14, 2016

@StefanRijnhart fixed checksum + active, transient is handled in Registry.__init__() and must be done before setup_models(), thanks for the comments.

@coleste coleste force-pushed the 9.0-base branch 2 times, most recently from ba800b9 to 0059b1a Compare January 14, 2016 18:34
#
# OpenUpgrade module for Odoo
# @copyright 2014-Today: Odoo Community Association
# @author: Sylvain LE GAL <https://twitter.com/legalsylvain>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace author by yourself here and elsewhere.

@StefanRijnhart
Copy link
Member

@coleste sorry for the delay, I missed your last message in my inbox. Thank you for the updates so far. Once you update the copyright and credits, it should be ready for merge.

@Yenthe666
Copy link

@StefanRijnhart he has added the copyright details. I think you can merge this one now? 🔹

@StefanRijnhart
Copy link
Member

Sorry, I was not aware. Thanks for the update and thanks for the comments everyone!

StefanRijnhart added a commit that referenced this pull request Feb 11, 2016
@StefanRijnhart StefanRijnhart merged commit 2d34f1a into OCA:9.0 Feb 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants