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

[IMP] base: add more information if KeyError #31958

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@fmdl
Copy link
Contributor

fmdl commented Mar 19, 2019

During a migration process or developpment if a deps is wrong, it is hard to find where is issue come.

same as #31889

@odony @nim-odoo

--
I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo robodoo added the seen 🙂 label Mar 19, 2019

@fmdl fmdl force-pushed the fmdl:patch-140 branch from cd84d74 to e48b6cc Mar 19, 2019

@fmdl fmdl force-pushed the fmdl:patch-140 branch from e48b6cc to 1b7fcb3 Mar 19, 2019

@@ -705,6 +705,8 @@ def resolve_deps(self, model, path0=[], seen=frozenset()):
_logger.warning("Field %s depends on itself; please fix its decorator @api.depends().", self)
model, path = model0, path0
for fname in dotnames.split('.'):
if fname not in model._fields:

This comment has been minimized.

Copy link
@odony

odony Mar 20, 2019

Contributor

same as #31889 : instead of an extra lookup in model._fields, how about using model._fields.get(name) below and raising if the result is None? It's not critical but it's not more code, so why make 2 lookups when we can make only 1?

This comment has been minimized.

Copy link
@fmdl

fmdl Mar 20, 2019

Author Contributor

because if you have some custom field, model._fields.[name] return None in some case.
the issue with get(), it return None if the key doesn't exist, and return also None is the value is None.

maybe me need to write some thing like this:

try:
    field = model._fields[name]
except KeyError:
    raise KeyError("Field : %s, wrong field '%s' of this deps : '%s'." % (self, fname, dotnames)

or

field = model._fields.get(name, 'keyerror_field_doesnt_exist')
if field == 'keyerror_field_doesnt_exist':
    raise KeyError("Field : %s, wrong field '%s' of this deps : '%s'." % (self, fname, dotnames)

@odony what is for you the best solution ?

This comment has been minimized.

Copy link
@fmdl

fmdl Mar 26, 2019

Author Contributor

ping @odony

Show resolved Hide resolved odoo/fields.py Outdated
Update odoo/fields.py
Co-Authored-By: fmdl <florent.mirieu@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.