Univention Bugzilla – Full Text Bug Listing |
Summary: | Traceback when UDM module is None (UDM_Error: 'NoneType' object has no attribute 'object') | ||
---|---|---|---|
Product: | UCS | Reporter: | Dirk Wiesenthal <wiesenthal> |
Component: | UMC - Domain management (Generic) | Assignee: | Florian Best <best> |
Status: | CLOSED FIXED | QA Contact: | Dirk Wiesenthal <wiesenthal> |
Severity: | normal | ||
Priority: | P5 | CC: | best, dormann, ebersbach, gohmann, klaeser, requate, walkenhorst |
Version: | UCS 3.2 | ||
Target Milestone: | UCS 4.0-0-errata | ||
Hardware: | Other | ||
OS: | Linux | ||
See Also: |
https://forge.univention.org/bugzilla/show_bug.cgi?id=29231 https://forge.univention.org/bugzilla/show_bug.cgi?id=34246 |
||
What kind of report is it?: | --- | 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): | Error handling, External feedback | |
Max CVSS v3 score: |
Description
Dirk Wiesenthal
2014-04-29 11:21:30 CEST
reported again, 3.2-1 errata111 (Borgfeld). Also reported with 3.2-3 errata185 (Borgfeld). Remark: Aufrufen eines Users Version: 3.2-3 errata206 (Borgfeld) svn r57078: Bug #34680: Fix errors when no module for specific ldap objects exists. Multiple checks contained these errors: Lines like the following were used often: "module = UDM_Module(udm_module); if module is None:" These could never be true, the correct check is "if module.module is None". The ModuleCache.get() method was wrong, it saved 'None' internally in some cases. A reload of modules then had no effect. get_module() returned UDM_Module() instances even if the handler module for them does not exists. (In reply to Florian Best from comment #5) > Lines like the following were used often: > "module = UDM_Module(udm_module); if module is None:" > These could never be true, the correct check is "if module.module is None". Please also consider "if module not None". Maybe implement __nonzero__ in class UDM_Module? module.module seems... awkward. Note that there is also a lot of: module = get_module(flavor, superordinate) if not module: MODULE.error('Idenfified module %s for %s (flavor=%s) does not have a relating UDM module.' % (ldap_dn, flavor, modules[0])) s/Idenfified/Identified/ - and I guess the argument ordering is wrong? Your change in def move is error prone (although probably without any consequences): if 'container' not in options: yield {'$dn$': object, 'success': False, 'details': _('The destination is missing')} module = get_module(None, object) if not module: # <----- BTW another faulty test yield {'$dn$': object, 'success': False, 'details': _('Could not identify the given LDAP object')} ... can yield twice in one for-iteration - if 'container' not in options. (In reply to Dirk Wiesenthal from comment #6) > (In reply to Florian Best from comment #5) > > Lines like the following were used often: > > "module = UDM_Module(udm_module); if module is None:" > > These could never be true, the correct check is "if module.module is None". > > Please also consider "if module not None". Maybe implement __nonzero__ in > class UDM_Module? module.module seems... awkward. Note that there is also a > lot of: > module = get_module(flavor, superordinate) > if not module: This is correct! get_module() will never return a module if module.module does not exists. Note that there were 2 different get_module functions: get_module() and self.get_module()(not anymore)/self.get_module_by_request(). > MODULE.error('Idenfified module %s for %s (flavor=%s) does not have a > relating UDM module.' % (ldap_dn, flavor, modules[0])) > > s/Idenfified/Identified/ - and I guess the argument ordering is wrong? yes, both has been fixed. > Your change in def move is error prone (although probably without any > consequences): > if 'container' not in options: > yield {'$dn$': object, 'success': False, 'details': _('The destination is > missing')} > module = get_module(None, object) > if not module: # <----- BTW another faulty test → not faulty, see the first comment. > yield {'$dn$': object, 'success': False, 'details': _('Could not identify > the given LDAP object')} > > ... can yield twice in one for-iteration - if 'container' not in options. yes, the 'continue' statement was missing. OK, the traceback is gone *** Bug 26966 has been marked as a duplicate of this bug. *** *** Bug 29201 has been marked as a duplicate of this bug. *** |