Univention Bugzilla – Bug 44786
Improve listener module API
Last modified: 2020-05-25 19:01:22 CEST
The listener module API makes reading and writing listener modules more cumbersome than necessary. * It pollutes the module (file) namespace for both configuration and code with hard coded attribute names. * → Adding configuration variables is dangerous, as a name in the module namespace might be in use in an existing module. * The signature of the central function changes depending on the setting of another variable. * Because all possible LDAP changes have to be handled by a single function call, every listener module repeats the same situation-classification code. * All listener modules and the listener server log together into the same file. * All listener modules and the listener server use the same logging level. * The modrdn switch makes it necessary for every programmer to write (the same) code to handle it. Create a listener module API that allows evolving the listener framework without endangering existing listener modules. The implementation should deliver common code (like modrdn and LDAP connection handling and logging) and have the framework call dedicated functions with proper names for different situations (create, modify, ...). Separate configuration from code to allow evolving them separately. Create an adapter to allow using the new API with the existing listener framework.
r80186: fork listener package r80187: add new listener module API
r80267: add UCR switch to deactivate listener module, add UCRV descriptions
(In reply to Daniel Tröder from comment #0) > * All listener modules and the listener server log together into the same > file. I see this as an advantage since the listener modules are processed sequentially and the order in which actions are performed by listener modules is sometimes relevant (provided that each listener module gets is own automatic logging prefix).
IMHO the main listener.log should only contain what it is doing, not what the modules are doing. Or at least only with log level >= ERROR. started LM1 finished LM1 started LM2 finished LM2 started LM3 ERROR in LM3 traceback of LM3 started LM4 ... And in separate log files for each LM all the stuff they have to say in all detail. Anyway, in the proposed API implementation the logging is done so that all output goes to both /v/l/u/listener.log (backwards compatible) as well as /v/l/u/listener_module/<name>.log. The loglevel for messages that are sent to listener.log is taken from listener/debug/level, the loglevel for listener_module/<name>.log is taken from listener/module/<name>/debug/level.
r80481: fix lost modify call, add ucs-test
This bug is tagged for UCS 4.2-3. You committed it into the 4.2-2 branch. Is this correct?
A new API for programming listener modules was added. The Univention Directory Listener (further down called "listener server") was not modified. To allow the use of the new module API while maintaining the old API for the listener server, an adapter class was also added. In /usr/share/doc/univention-directory-listener/examples two examples can be found. The simple one, which can be used for quickly creating simple listener modules looks like this: -------------------------------------------------------------------- from univention.listener import ListenerModuleAdapter, ListenerModuleHandler, ListenerModuleConfiguration class ListenerModuleTemplate(ListenerModuleHandler): def create(self, dn, new): self.logger.debug('dn=%r', dn) def modify(self, dn, old, new, old_dn): self.logger.debug('dn=%r old_dn=%r', dn, old_dn) if old_dn: self.logger.info('it is (also) a move') def remove(self, dn, old): self.logger.debug('dn=%r', dn) class ListenerModuleTemplateConfiguration(ListenerModuleConfiguration): name = 'listener module template' description = 'a listener module template' ldap_filter = '' attributes = [] listener_module_class = ListenerModuleTemplate globals().update(ListenerModuleAdapter(ListenerModuleTemplateConfiguration()).get_globals()) -------------------------------------------------------------------- A Python logging object is available at self.logger. Two handlers are registered: * One is an adapter for univention.debug, sending messages to /v/l/u/listener.log (together with all other listener modules). The loglevel is thus the same as for all: UCRV listener/debug/level * One is a TimedRotatingFileHandler that will write to a separate file for each module with a separate loglevel for each module: /v/l/u/listener_module/<name>.log Two new UCRVs: * listener/module/<name>/deactivate: bool: deactivate a module (without removing it) * listener/module/<name>/debug/level: int: set the loglevel for messages going to /v/l/u/listener_module/<name>.log Both UCRVs require a restart of the listener server. It would be possible to change this. If desired please reopen. It would be inconsistent with the usual behavior of listener modules though. 00951c1c: add improved listener module API and test 334a44c5: add doc strings, only open LDAP connection when used e164a6c2: user 'listener' unknown before installation of package itself 622002a4: advisory update 228047a7: fix typo 52f9b42b: style fixes (thanks fbest) 76f60003: advisory update univention-directory-listener 11.0.1-31 ucs-test 7.0.23-72
b227ae6b: fix typo 5e465f4a9d: advisory update univention-directory-listener 11.0.1-32
f83142f7: add uldap position object to handler class b3c7ee9f: impove test coverage univention-directory-listener 11.0.1-33 ucs-test 7.0.23-75
caa13c5123: complete metadata attribute list, fix old-new comparison univention-directory-listener 11.0.1-34
ucs-test/tests/10_ldap/41listener_module_api_test.py fails: http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/59/SambaVersion=s3,Systemrolle=slave/testReport/junit/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/59/SambaVersion=s4,Systemrolle=backup/testReport/junit/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/59/SambaVersion=s4,Systemrolle=member/testReport/junit/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/59/SambaVersion=s4,Systemrolle=slave/testReport/junit/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/60/SambaVersion=s3,Systemrolle=member/testReport/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/60/SambaVersion=s4,Systemrolle=backup/testReport/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/60/SambaVersion=s4,Systemrolle=slave/testReport/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/61/SambaVersion=s4,Systemrolle=master/testReport/10_ldap/41listener_module_api_test/test/ http://jenkins.knut.univention.de:8080/job/UCS-4.2/job/UCS-4.2-2/job/AutotestJoin/61/SambaVersion=s3,Systemrolle=slave/testReport/10_ldap/41listener_module_api_test/test/
Already fixed this morning (trying to at least - it's a timing issue of the test): 724c84fb
Added static type checking. See https://git.knut.univention.de/univention/ucs/blob/4.2-2/management/univention-directory-listener/README.md on how to use it.
* The code handling LDAP credentials transfer (setdata()) was fixed, so it can handle notifier restarts and LDAP upstream server changes. * Support for creating asynchronous listener modules using the exact same simple API was added. * Tests for asynchronous listener modules with concurrency=1 and concurrency=5 were added. ac81260d: handle notifier restarts, pep8, extract exceptions b3102c88: add support for creating asynchronous listener modules using the same simple API d46a10bb: add tests for the asynchronous listener modules API 07eebd16: advisory update univention-directory-listener 11.0.1-39A~4.2.0.201711200538 ucs-test 7.0.23-94A~4.2.0.201711200543
* The API was simplified by moving the configuration into the handler class and modifying the modules namespace with the help of a meta class. The configuration class can be a simple class with the required attributes (will suffice in most cases) or a subclass of ListenerModuleConfiguration for advanced scenarios. * PEP 484 type hints were added to stub files to clean up the imports in the regular Python modules. A minimalistic listener module now looks like this: --------------------------------------------------------------------- from __future__ import absolute_import from univention.listener import ListenerModuleHandler class ListenerModuleTemplate(ListenerModuleHandler): class Configuration: name = 'unique_name' description = 'listener module description' ldap_filter = '(&(objectClass=inetOrgPerson)(uid=example))' attributes = ['sn', 'givenName'] def create(self, dn, new): self.logger.debug('dn: %r', dn) def modify(self, dn, old, new, old_dn): self.logger.debug('dn: %r', dn) if old_dn: self.logger.debug('it is (also) a move! old_dn: %r', old_dn) self.logger.debug('changed attributes: %r', self.diff(old, new)) def remove(self, dn, old): self.logger.debug('dn: %r', dn) --------------------------------------------------------------------- commit 04000a46a4d28abda799e4bc5dffc493d5dacc6a Bug #44786: move logger to handler class, make logger class better mypy parsable commit b5f0210101bf222147c8b2c5151da7928cba8d8d Bug #44786: make celery config update safer commit 158cb7f7d0d7ce90ab276152bcab6a8257616cda Bug #44786: simplify configuration, hide adapter usage commit b100b8a193f0672e0743a6d9833d34e88e767e92 Bug #44786: adapt examples commit d9e42129bca3fa40d37f2048e6fc9fbdf609c6c1 Bug #44786: adapt tests commit e64e00bbb92e67ee02b4cf161343185d19e4f9aa Bug #44786: move PEP 484 (type hints) to stub files commit 135ce412cd6e6b4937a4b1358519bf879616f85a Bug #44786: changelog commit 7ad6ddb1253351d3e9bb07eca79bd4e2f077e199 Bug #44786: update README, fix examples commit 9c8c206e95bf0a8a9bdf09e40cc7631ba97889df Bug #44786: advisory update Merged and build to: Branch: ucs_4.2-0 Scope: errata4.2-3 univention-directory-listener (11.0.1-41) ucs-test (7.0.23-109)
Merge missing commits to 4.3 branch: beed68c4 (cherry picked from commit 04000a4) 8d627aaf (cherry picked from commit b5f0210) da5cd3ea (cherry picked from commit 158cb7f) 896db010 (cherry picked from commit b100b8a) 3b3e854d (cherry picked from commit d9e4212) 6d8e2b7f (cherry picked from commit e64e00b) 9ec581b1 (cherry picked from commit 7ad6ddb) f41c23a8 Bug #44786: changelog
[4.2-3 950b138440] Bug #44786: check type of arguments [4.2-3 320e7f79e7] Bug #44786: changelog [4.2-3 d26b7dc020] Bug #44786: fix typo [4.2-3 110646e341] Bug #44786: changelog [4.2-3 942a99e75c] Bug #44786: advisory update univention-directory-listener 11.0.1-43A~4.2.0.201801091005 [4.3-0 d117ed93f2] Bug #44786: check type of arguments [4.3-0 916003a33b] Bug #44786: changelog [4.3-0 37a536c0f2] Bug #44786: fix typo [4.3-0 65e7839bf1] Bug #44786: changelog univention-directory-listener 12.0.0-12A~4.3.0.201801091002 [4.3-0 58826c2483] Bug #44786: changelog ucs-test 8.0.7-8A~4.3.0.201801090857
Patch to massively boost query performance of the main listener process: use an indexed attribute. [4.2-3 baf9401085] Bug #44786: use indexed attribute univention-directory-listener 11.0.1-44A~4.2.0.201801120808 --- [4.3-0 328f1d2d01] Bug #44786: use indexed attribute univention-directory-listener 12.0.0-13A~4.3.0.201801120815
There is a serious questionable thing in the API. Dirk ran into the problem: univention.listener.ListenerModuleHandler.po is a @property, which always returns a new property instance. class Foo(ListenerModuleHandler): def create(self): self.po.setDn('zoneName=asdf,%s' % (ldap_base,)) print self.po.getDn() → prints the ldap base instead of the zoneName. A property() instance should never have side effect. It would be a better API if it would be a method get_position().
[4.2-3 4494d911801] Bug #44786: exception hierarchy, clear LDAP credentials for server updates, remove UDM code [4.2-3 3f63021ecd3] Bug #44786: changelog [4.3-0 638ae0b9b1f] Bug #44786: exception hierarchy, clear LDAP credentials for server updates, remove UDM code [4.3-0 9501e0b287f] Bug #44786: changelog [4.2-3 6cc8219eb9b] Bug #44786: fix mypy hints file [4.3-0 b368ff34f18] Bug #44786: fix mypy hints file Package: univention-directory-listener Version: 11.0.1-46A~4.2.0.201801221622 Branch: ucs_4.2-0 Scope: errata4.2-3 Package: univention-directory-listener Version: 12.0.0-15A~4.3.0.201801221554 Branch: ucs_4.3-0
[4.2-3 b7ec2f15940] Bug #45968: remove dynamicmaps.cf only if it contains old paths ucs-test (7.0.23-117)
To enable the quick release of an erratum the code in this bug has been temporarily deactivated. @QA: please inform me when I should reactivate the code. [4.3-0 bcd2b923ac] Bug #44786: temporarily remove new listener module API from binary package [4.3-0 4b7af540eb] Bug #44786: temporarily deactivate tests for new listener module API [4.3-0 ebf8490562] Bug #44786: changelog, remove advisory [4.2-3 fe174c8cfa] Bug #44786: temporarily remove new listener module API from binary package [4.2-3 1f8df9d975] Bug #44786: temporarily deactivate tests for new listener module API [4.2-3 bc3b8bb0cd] Bug #44786: changelog, advisory [4.2-3 cefaa4425d] Bug #44786: advisory update univention-directory-listener (11.0.1-48) univention-directory-listener (12.0.0-18)
Ooops, updating the yaml like this results in a dangerous situation: * check_errata_for_release checks the version number and the status of the bugs referenced by the advisory. It doesn't detect that the package has been rebuilt and this bug is still without QA. As a result we would nearly have announced it now -- without QA. That's a flaw in our release procedure. I think we should do it in reverse order in this case: The bug number must not be removed from an advisory before the Bug is verified.
A change like this should happen in a git branch. If git is good for anything in our work flow than it would be this.
In the mean time git history has been re-written, so the commit hashes noted at this bug cannot be used to identify the commits that needed reverting. What about this one e.g: ===================================================================== Merge: 29e0b68 219c8e3 Author: Daniel Troeder <troeder@univention.de> Date: Wed Oct 25 15:48:01 2017 +0200 Bug #44786: Merge branch 'dtroeder/44786_listener_api' into 4.2-2 ===================================================================== Please suggest a way to get out of this situation. Quickly.
Ok, as suggested by the assignee I've compared the source packages in the scopes: arequate@dimma:~$ pkginfo-build2 --published univention-directory-listener \ | grep ucs_4.2-0-errata Version: 11.0.1-27A~4.2.0.201704251445 ucs_4.2-0-errata4.2-0 arequate@dimma:~$ pkginfo-build2 univention-directory-listener \ | grep ucs_4.2-0-errata4.2-3 Version: 11.0.1-48A~4.2.0.201802281040 ucs_4.2-0-errata4.2-3 arequate@dimma:~$ dpkg-source -x /var/univention/buildsystem2/mirror/ftp/4.2/unmaintained/component/4.2-0-errata/source/univention-directory-listener_11.0.1-27A~4.2.0.201704251445.dsc orig arequate@dimma:~$ dpkg-source -x /var/univention/buildsystem2/apt/ucs_4.2-0-errata4.2-3/source/univention-directory-listener_11.0.1-48A~4.2.0.201802281040.dsc qa arequate@dimma:~$ LANG=C diff -x changelog -uar orig qa | grep -v ^Only => Output looks ok.
* The new listener module API has been reactivated in the UCS 4.2-3 errata scope and in 4.3-0. * The async listener module API has been removed from both branches. [4.3-0 13261bf9a4] Bug #44786: reactivate new listener module api (partial revert of bcd2b923ac) [4.3-0 d8b74e7104] Bug #44786: reactivate new listener module api test (partial revert of 4b7af540eb) [4.3-0 19877bf850] Bug #44786: changelog [4.3-0 1d2d5d5598] Bug #44786: remove async listener module API [4.3-0 628be71289] Bug #44786: changelog [4.3-0 c196957f6f] Bug #44786: remove async listener module API tests [4.3-0 bc9261460d] Bug #44786: changelog univention-directory-listener (12.0.0-19) ucs-test (8.0.28-60) [4.2-3 5033d64434] Bug #44786: reactivate new listener module api (partial revert of bcd2b923ac) [4.2-3 3aa8c56f23] Bug #44786: reactivate new listener module api (partial revert of bcd2b923ac) [4.2-3 12567e37b8] Bug #44786: reactivate new listener module api test (partial revert of 4b7af540eb) [4.2-3 6a9569971f] Bug #44786: remove async listener module API [4.2-3 900245f364] Bug #44786: changelog [4.2-3 0a8bb4c269] Bug #44786: advisory (12567e37b8 was by accident, it is an empty commit.) univention-directory-listener (11.0.1-49) ucs-test (7.0.23-120) * The async listener module API code is available in the branch dtroeder/async_listener_api, which was branched after 19877bf850. [dtroeder/async_listener_api 8115b6a7b1] Bug #44786: reactivate async listener module API
Works great in UCS 4.2 and 4.3
<http://errata.software-univention.de/ucs/4.2/311.html>