Bug 41266 - remove dead code from custom-attributes
remove dead code from custom-attributes
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: UDM (Generic)
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.1-2-errata
Assigned To: Florian Best
Philipp Hahn
:
: 31769 (view as bug list)
Depends on:
Blocks: 41553 41554 41556
  Show dependency treegraph
 
Reported: 2016-05-12 10:04 CEST by Florian Best
Modified: 2016-09-29 17:30 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.069
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Ticket number:
Bug group (optional): API change, Cleanup
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Best univentionstaff 2016-05-12 10:04:29 CEST
The migration from custom attributes to extended attributes partly failed.
The old univentionAdminProperty*-attributes have been renamed into univentionUDMProperty*.

Some code was not migrated to use the new attributes, which leads to never executed code:

modules.py:     for dn, attrs in lo.search(base=position.getDomainConfigBase(), filter='(&(objectClass=univentionAdminProperty)(univentionAdminPropertyModule=%s))' % name(module)):
modules.py:             propertySyntaxString=attrs.get('univentionAdminPropertySyntax', [''])[0]
modules.py:             propertyDefault = attrs.get('univentionAdminPropertyDefault', [''])[0]
modules.py:             if attrs.get('univentionAdminPropertyMultivalue', [''])[0] == '1':
modules.py:                     short_description=attrs['univentionAdminPropertyShortDescription'][0],
modules.py:                     long_description=attrs.get('univentionAdminPropertyLongDescription',[''])[0],
modules.py:             if attrs['univentionAdminPropertyLdapMapping'][0].upper() != 'ObjectClass'.upper():
modules.py:                     module.mapping.register(pname, attrs['univentionAdminPropertyLdapMapping'][0], None, map_method)
modules.py:                     module.mapping.register(pname, attrs['univentionAdminPropertyLdapMapping'][0], univention.admin.mapping.nothing, univention.admin.mapping.nothing)
modules.py:             deleteValues=attrs.get('univentionAdminPropertyDeleteValues')
modules.py:             tabname = attrs.get('univentionAdminPropertyLayoutTabName',[_('Custom')])[0]
modules.py:             if attrs.get('univentionAdminPropertyDeleteObjectClass'):
modules.py:                     deleteObjectClass=attrs.get('univentionAdminPropertyDeleteObjectClass')[0]
modules.py:             tabposition = attrs.get('univentionAdminPropertyLayoutPosition',['-1'])[0]
modules.py:             module.ldap_extra_objectclasses.extend( ([(attrs.get('univentionAdminPropertyObjectClass', [])[0], pname, propertySyntaxString, attrs['univentionAdminPropertyLdapMapping'][0], deleteValues, deleteObjectClass )]))

We should remove the code or reintegrate the broken/missing features.
Comment 1 Florian Best univentionstaff 2016-05-24 19:59:56 CEST
Removed that code during working on Bug #25240. It costs a lot of time to always read broken code.

univention-directory-manager-modules (11.0.2-30):
r69513 | Bug #41266: remove dead custom-attributes code

univention-directory-manager-modules.yaml:
r69515 | YAML Bug #41266 Bug #25240
Comment 2 Florian Best univentionstaff 2016-05-27 13:06:53 CEST
*** Bug 31769 has been marked as a duplicate of this bug. ***
Comment 3 Florian Best univentionstaff 2016-05-27 13:07:26 CEST
r69538 | Bug #41266: removed some more unused custom-attributes code
Comment 4 Philipp Hahn univentionstaff 2016-06-06 13:31:16 CEST
(In reply to Florian Best from comment #1)
> Removed that code during working on Bug #25240. It costs a lot of time to
> always read broken code.
> 
> univention-directory-manager-modules (11.0.2-30):
> r69513 | Bug #41266: remove dead custom-attributes code

REOPEN: Please remove the code in _ldap_modlist() completely instead of out-commenting it.

> univention-directory-manager-modules.yaml:
> r69515 | YAML Bug #41266 Bug #25240

OK


(In reply to Florian Best from comment #3)
> r69538 | Bug #41266: removed some more unused custom-attributes code

OK


(In reply to Florian Best from comment #2)
> *** Bug 31769 has been marked as a duplicate of this bug. ***

REOPEN: More CA related code to be removed in
 etc/bash_completion.d/univention-directory-manager
 test/univention-admin-test
 modules/univention/admin/__init__.py
Comment 5 Florian Best univentionstaff 2016-06-08 16:17:41 CEST
(In reply to Philipp Hahn from comment #4)
> REOPEN: Please remove the code in _ldap_modlist() completely instead of
> out-commenting it.
That code contains information which are still valid and should be reimplemented some day.

> REOPEN: More CA related code to be removed in
>  etc/bash_completion.d/univention-directory-manager
Yes, fixed.
>  test/univention-admin-test
That scripts seems broken, the LDAP server doesn't start after applying the schema in there. Nevertheless is converted custom-attributes → extended attributes in there.
>  modules/univention/admin/__init__.py
I don't see Code in there which still refers to custom attributes (only two comments which have been adjusted).

univention-directory-manager-modules (11.0.3-2):
r69968 | Bug #41266: remove more custom attribute related code
Comment 6 Philipp Hahn univentionstaff 2016-06-08 18:23:01 CEST
(In reply to Florian Best from comment #5)
> (In reply to Philipp Hahn from comment #4)
> > REOPEN: Please remove the code in _ldap_modlist() completely instead of
> > out-commenting it.
> That code contains information which are still valid and should be
> reimplemented some day.

Such commented out code tends to get stale very quickly because nobody will touch it or keep it up-to-date.
We have source code control for archaeologists, so please remove it.
Comment 7 Philipp Hahn univentionstaff 2016-06-09 08:21:00 CEST
(In reply to Florian Best from comment #5)
> (In reply to Philipp Hahn from comment #4)
> > REOPEN: Please remove the code in _ldap_modlist() completely instead of
> > out-commenting it.
> That code contains information which are still valid and should be
> reimplemented some day.

Such commented out code tends to get stale very quickly because nobody will
touch it or keep it up-to-date.
The source code is no TODO-list or issue tracking system; please file a new bug to re-implement it if necessary; then management can decide on the importance.
We also have source code control if someone wants to lookup old code, so please remove it.
Comment 8 Florian Best univentionstaff 2016-06-09 11:55:22 CEST
Your wish is my command.

univention-directory-manager-modules (11.0.3-3):
r70014 | Bug #41266: remove commented out code
Comment 9 Philipp Hahn univentionstaff 2016-06-14 12:22:07 CEST
OK: r69968 r70014
TODO: Bug #41553, Bug #41554, Bug #41556
FIXED: errata-announce -V univention-directory-manager-modules.yaml
 r70158 | Bug #41266 udm-modules: YAML v++

OK: EA & EO still work
Comment 10 Janek Walkenhorst univentionstaff 2016-06-15 16:20:39 CEST
<http://errata.software-univention.de/ucs/4.1/199.html>