Bug 28276 - configRegistry[key] does not raise KeyError
configRegistry[key] does not raise KeyError
Status: REOPENED
Product: UCS
Classification: Unclassified
Component: UCR
UCS 5.0
All Linux
: P5 enhancement (vote)
: ---
Assigned To: UCS maintainers
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-08-22 13:27 CEST by Philipp Hahn
Modified: 2020-07-13 20:06 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): API change, Further conceptual development
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Philipp Hahn univentionstaff 2012-08-22 13:27:43 CEST
univention.config_registry.ConfigRegistry.__getitem__(self, key) is just a wrapper for get(self, key, default=None), but does not raise a KeyError, which a Python dictionary should do; to quote from <http://docs.python.org/reference/datamodel.html#object.__getitem__>:
... For mapping types, if key is missing (not in the container), KeyError should be raised.

This is unexpected and complicates differenciating unset values from empty values using Exceptions.
Some code even defines its own wrapper to get the default dictionary behaviour:

def ucr(key, default=None):
    """Wrapper around configRegistry throwing KeyErrors on undefined keys."""
    if default is None:
        default = ucr.UNIQUE
    value = configRegistry.get(key, default)
    if value is ucr.UNIQUE:
        raise KeyError(key)
    return value
ucr.UNIQUE = object()

IMHO the behaviour of UCR should be changed to throw Exceptions.

See Bug #27465 for a similar problem in the command line tool.
Comment 1 Philipp Hahn univentionstaff 2012-08-22 14:16:20 CEST
Within univention.config_registry.interfaces VengefulConfigRegistry is used to wrap the ConfigRegistry instance to throw KeyErrors instead of returning '' for unset keys.
This fixes the error of ipv6_gateway() always returning an error even is the value is unset.

svn34995, univention-config-registry_8.0.2-4.394.201208221410

But the decision to change the default behaviour to generally throw a KeyError remains.
Comment 2 Stefan Gohmann univentionstaff 2012-08-22 20:27:48 CEST
(In reply to comment #1)
> But the decision to change the default behaviour to generally throw a KeyError
> remains.

Yes, it makes sense. It is an API change, so UCS 4 would be the next version for this issue.
Comment 3 Stefan Gohmann univentionstaff 2014-06-13 16:50:49 CEST
(In reply to Stefan Gohmann from comment #2)
> (In reply to comment #1)
> > But the decision to change the default behaviour to generally throw a KeyError
> > remains.
> 
> Yes, it makes sense. It is an API change, so UCS 4 would be the next version
> for this issue.

In general it would be useful. But I think the benefit is less than the cost. It has to be changed in our and in all external code and we have to support both for a time.

Removed UCS 4 target milestone.
Comment 4 Ingo Steuwer univentionstaff 2020-07-03 20:54:26 CEST
This issue has been filed against UCS 4.2.

UCS 4.2 is out of maintenance and many UCS components have changed in later releases. Thus, this issue is now being closed.

If this issue still occurs in newer UCS versions, please use "Clone this bug" or reopen it and update the UCS version. In this case please provide detailed information on how this issue is affecting you.