-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[16.0][ADD] module_auto_update: allow updates in a minimal environment #2604
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
Conversation
|
Thanks! Can you squash the fixup commit? |
c20fc84 to
3daf833
Compare
|
There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually 3 models make problems: res.users, res.partner and res.company. For res.company I only saw it happen only a few times depending on some field definitions. res.users is fixed with that one. However res.partner still causes issues. Because it happens only occasionally the workaround was enough. The reason behind this is the context_get call for the shell which tries to load a context from an user because of prefetch_fields it loads more than necessary (lang + tz) from the database causing SQL exceptions.
A way to solve it would be to hook into it and somehow signal that we don't want to prefetch the fields. If environment variables like below are the best solution is up for discussion but the following code seems to work for res.partner, res.users and res.company without the fix in this PR. The shell was started with:
ODOO_NO_PREFETCH_FIELDS=1 ./odoo-bin shell -c ../../etc/odoo.cfgfrom odoo import api
_original_environment_new = api.Environment.__new__
def environment_new(cls, cr, uid, context, *args, **kwargs):
if os.environ.get("ODOO_NO_PREFETCH_FIELDS"):
context = dict(context or {}, prefetch_fields=False)
return _original_environment_new(cls, cr, uid, context, *args, **kwargs)
api.Environment.__new__ = environment_newWhat do you think about it? Do you think it's valid to use an environment variable or do you know a better way? The monkey patch is sadly not great. A different way would be to overwrite context_get of res.users and do it similarly. I look forward to it. The PR is a good solution for res.users without invoking the shell differently and monkey patching Environment.
|
@fkantelberg which issues do field additions to res.partner cause in this context? I've used this with many changes to res.partner and never encountered a problem. Is your proposal an alternative to this PR? It's unclear to me what your use case is, so I can't really form an opinion about env var versus extra parameter, config key, move the code to a module added to --load, ... |
|
If you add a field in an already installed module on My current guess is: ./odoo-bin shell -c ../../etc/odoo.cfg |
|
hmm, my commandline is ocb/odoo-bin shell --addons-path ocb/addons,ocb/odoo/addons,custom-addons -d odoo16 --smtp False --http-interface localhost --http-port 42420 --limit-time-real 6000 $* <<ODOODOC
env['ir.module.module'].upgrade_changed_checksum_shell()
ODOODOCand this works to update both res.user and res.partner changes |
|
haha, because I've merged in hbrunn/OCB@eb35a10 in my projects for this reason :-) Well, indeed we'll need to patch this away somehow. I guess making this a real command as in https://github.com/OCA/OCB/blob/16.0/odoo/cli/command.py would be the cleanest way to do this? |
|
True that would be a nice option and we could initialize the environment with an empty context. I would love this solution even more. |
|
Aside from how this is going to be solved here, would you like to update that PR to work against master @hbrunn? And then maybe we can all throw our weight behind it to get it merged. |
|
yes, I'm not going to touch this PR. Meanwhile adding a class in import odoo
from odoo import api, SUPERUSER_ID
from odoo.cli.command import Command
from odoo.cli.shell import Shell
from odoo.tools import config
class ModuleAutoUpgrade(Command):
"""Run auto upgrade"""
def run(self, args):
Shell.init(self, args)
registry = odoo.registry(config['db_name'])
with registry.cursor() as cr:
minimal_env = api.Environment(cr, SUPERUSER_ID, {'prefetch_fields': False})
minimal_env["ir.module.module"].upgrade_changed_checksum(
overwrite_existing_translations=config['overwrite_existing_translations'],
)seems to do the trick, now I can say ocb/odoo-bin --addons-path=ocb/addons,ocb/odoo/addons,custom-addons moduleautoupgrade -d odoo16to trigger a module update that's run in a bare environment. (it matters that With the interesting part done now I'll put this on my backlog for picking up eventually, but will happily review if anybody wants to continue from here :-) |
|
Wasn't quite working like that (maybe because your patch was still deployed in your environment) but #3487 works nicely. The |
|
superseded by #3487 |
without this, updates fail when you add columns to models that are used here, namely
res.usersandir.model