Projects
Wiki     Timeline     Roadmap     Browse Source     View Tickets     New Ticket     Search

Ticket #136 (new Defect)

Opened 6 years ago

Last modified 13 months ago

Can't use non-default directory service

Reported by: tylerkeating@… Owned by: sagen@…
Priority: 1: Blocker Milestone: Later
Component: Calendar Server Severity: Crash/data loss
Keywords: Cc:
Port:

Description (last modified by wsanchez@…) (diff)

Hi, After a recent update, I'm now getting the following error when trying to run the server.

Traceback (most recent call last):

File "/Users/admin/Developer/Collaboration/Twisted/bin/twistd", line 21, in <module>
run()

File "/Users/admin/Developer/Collaboration/Twisted/twisted/scripts/twistd.py", line 27, in run
app.run(runApp, ServerOptions)

File "/Users/admin/Developer/Collaboration/Twisted/twisted/application/app.py", line 374, in run
config.parseOptions()

File "/Users/admin/Developer/Collaboration/Twisted/twisted/application/app.py", line 354, in parseOptions
usage.Options.parseOptions(self, options)

File "/Users/admin/Developer/Collaboration/Twisted/twisted/python/usage.py", line 189, in parseOptions
self.subOptions.parseOptions(rest)

File "/Users/admin/Developer/Collaboration/Twisted/twisted/python/usage.py", line 199, in parseOptions
self.postOptions()

File "/Users/admin/Developer/Collaboration/CalendarServer/twistedcaldav/tap.py", line 127, in postOptions
parseConfig(self['config'])

File "/Users/admin/Developer/Collaboration/CalendarServer/twistedcaldav/config.py", line 342, in parseConfig
config.loadConfig(configFile)

File "/Users/admin/Developer/Collaboration/CalendarServer/twistedcaldav/config.py", line 242, in loadConfig
self.update(configDict)

File "/Users/admin/Developer/Collaboration/CalendarServer/twistedcaldav/config.py", line 186, in update
self._data["DirectoryService"]["params"] = copy.deepcopy(serviceDefaultParams[dsType])

KeyError: 'twistedcaldav.directory.myapp.MyAppDirectoryService'

Where 'twistedcaldav.directory.myapp.MyAppDirectoryService' is my custom directory service, which obviously isn't in the list of serviceDefaultParams. Since serviceDefaultParams only includes XML and Open directory services, you get the same error trying to use the Basic, Digest or SQL directory services too. There is something wrong with how the update method is parsing directory services.

  • Tyler

Attachments

config.py (11.7 KB) - added by tylerkeating@… 6 years ago.
config.py.patch (2.1 KB) - added by callan@… 6 years ago.
Patch to allow directory service implementations to declare their default configuration options.
extensible_config.patch (15.7 KB) - added by callan@… 6 years ago.
More ambitious patch extending the extensible configuration logic throught
extensible_config_v2.patch (19.2 KB) - added by callan@… 6 years ago.
After being reminded via e-mail by several people of the existence of other DirectoryService instances such as the ApacheDirectoryService I've made sure each of these has defaultParams and a test to ensure that it actually loads correctly.
extensible_config_v3.patch (20.6 KB) - added by callan@… 6 years ago.
Updated patch with test cases covering the possibility of dictionary based parameters
extensible_config_v4.patch (20.6 KB) - added by callan@… 6 years ago.
Further dictionary test fixes

Change History

comment:1 Changed 6 years ago by wsanchez@…

  • Priority changed from 5: Not set to 1: Blocker
  • Owner changed from wsanchez@… to dreid@…
  • Severity changed from Serious to Crash/data loss
  • Milestone set to Preview 1

Whoops, that's no good.

comment:2 Changed 6 years ago by wsanchez@…

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in r1493.

comment:3 Changed 6 years ago by wsanchez@…

Note ticket #138 which requests that we make this extensible.

Changed 6 years ago by tylerkeating@…

comment:4 Changed 6 years ago by tylerkeating@…

  • Status changed from closed to reopened
  • Resolution fixed deleted

There is still a bug in the code and using anything other than the XML or OpenDirectory directory service will result in a crash. To simulate it, you only need to uncomment the default Apache-style directory service included in caldavd-text.plist.

Just to get it running on my end, I've moved the offending clauses under the new conditional checking for a default directory service and attached the file (see previous attachment). Please have a look at it, it's a minor change.

comment:5 Changed 6 years ago by wsanchez@…

  • Description modified (diff)

comment:6 Changed 6 years ago by wsanchez@…

  • Milestone changed from Preview 1 to Version One

comment:7 Changed 6 years ago by wsanchez@…

  • Milestone changed from 1.0 to 2.0

comment:8 Changed 6 years ago by callan@…

Having written a (hackish) solution to this for my own uses and come across dreid in IRC I've cleaned it up and attach a patch to allow this to be extensible. The basic premise is to load the class using the Twisted reflection utilities, look for a defaultParams class variable and inject it into the defaultServiceParams module variable in twistedcaldav.config. It also provides sanity checks for class loading and altogether missing default configuration parameters. For example:

class MyDirectoryService(DirectoryService):
    defaultParams = {
        'constructorVariable1': 'James Bond 007'
        'constructorVariable2': 'Ian Fleming'
    }
    ...

If preferable I can also do it using a class method:

class MyDirectoryService(DirectoryService):
    @classmethod
    def getDefaultParams(klass):
        return {
            'constructorVariable1': 'James Bond 007'
            'constructorVariable2': 'Ian Fleming'
        }
    ...

Comments? This should also resolve #138.

-Chris (kagon)

Changed 6 years ago by callan@…

Patch to allow directory service implementations to declare their default configuration options.

comment:9 Changed 6 years ago by callan@…

After further discussions with dreid the band-aid style patch idea was thrown out and a new slightly more ambitious idea was put forward. This new patch should ensure all existing DirectoryService instances follow the extensibility paradigm discussed above and were realized by the initial patch. The new patch includes the following changes:

  • Removal of serviceDefaultParams altogether
  • Changes to twistedcaldav.config.Config.update() to expect all default configuration options to be loaded at runtime
  • Addition of defaultParams to the IDirectoryService interface
  • Extension of twistedcaldav.test.test_config to encompass the new logic
  • Changes to twistedcaldav.test.test_config to allow a little more modularity and code reuse

Changed 6 years ago by callan@…

More ambitious patch extending the extensible configuration logic throught

comment:10 Changed 6 years ago by callan@…

With the patch applied all the tests pass apart from those I cannot run due to being on Linux:

callan@shadowblade ~/CalendarServer $ trial twistedcaldav.test.test_config
Running 22 tests.
twistedcaldav.test.test_config
  ConfigTests
    testCopiesDefaults ...                                                 [OK]
    testDefaults ...                                                       [OK]
    testDirectoryService_badParam ...                                      [OK]
    testDirectoryService_newBadParam ...                                   [OK]
    testDirectoryService_newParam ...                                      [OK]
    testDirectoryService_newParamOD ...                                 [ERROR]
    testDirectoryService_newType ...                                       [OK]
    testDirectoryService_newTypeOD ...                                  [ERROR]
    testDirectoryService_noChange ...                                      [OK]
    testDirectoryService_sameType ...                                      [OK]
    testDirectoryService_twoNewParams ...                                  [OK]
    testDirectoryService_unknownType ...                                   [OK]
    testLoadConfig ...                                                     [OK]
    testMerge ...                                                          [OK]
    testMergeDefaults ...                                                  [OK]
    testReloading ...                                                      [OK]
    testScoping ...                                                        [OK]
    testSetAttr ...                                                        [OK]
    testSetDefaults ...                                                    [OK]
    testUpdateAndReload ...                                                [OK]
    testUpdateDefaults ...                                                 [OK]
    testUpdating ...                                                       [OK]

===============================================================================
[ERROR]: twistedcaldav.test.test_config.ConfigTests.testDirectoryService_newParamOD

Traceback (most recent call last):
  File "/home/callan/CalendarServer/twistedcaldav/test/test_config.py", line 249, in testDirectoryService_newParamOD
    config.update({"DirectoryService": {"type": OPEN_DIRECTORY_SERVICE}})
  File "/home/callan/CalendarServer/twistedcaldav/config.py", line 204, in update
    dsClass = _loadDirectoryService(dsType)
  File "/home/callan/CalendarServer/twistedcaldav/config.py", line 35, in _loadDirectoryService
    raise ConfigurationError(
twistedcaldav.config.ConfigurationError: Unable to load directory service twistedcaldav.directory.appleopendirectory.OpenDirectoryService
===============================================================================
[ERROR]: twistedcaldav.test.test_config.ConfigTests.testDirectoryService_newTypeOD

Traceback (most recent call last):
  File "/home/callan/CalendarServer/twistedcaldav/test/test_config.py", line 244, in testDirectoryService_newTypeOD
    config.update({"DirectoryService": {"type": OPEN_DIRECTORY_SERVICE}})
  File "/home/callan/CalendarServer/twistedcaldav/config.py", line 204, in update
    dsClass = _loadDirectoryService(dsType)
  File "/home/callan/CalendarServer/twistedcaldav/config.py", line 35, in _loadDirectoryService
    raise ConfigurationError(
twistedcaldav.config.ConfigurationError: Unable to load directory service twistedcaldav.directory.appleopendirectory.OpenDirectoryService
-------------------------------------------------------------------------------
Ran 22 tests in 0.105s

FAILED (errors=2, successes=20)

Changed 6 years ago by callan@…

After being reminded via e-mail by several people of the existence of other DirectoryService instances such as the ApacheDirectoryService I've made sure each of these has defaultParams and a test to ensure that it actually loads correctly.

comment:11 Changed 6 years ago by dreid@…

  • Status changed from reopened to new
  • Owner changed from dreid@… to callan@…

config.py:33 You no longer have the possibility of an AttributeError

What if the values of one of my parameters is a dictionary, do we want to be able to merge the defaults with what is specified in the config file? Does this work already?

comment:12 Changed 6 years ago by callan@…

config.py:33 You no longer have the possibility of an AttributeError

Untrue unfortunately, and I've already been bitten by a misspelling that raised one. For example:

callan@shadowblade ~ $ python
Python 2.5.1 (r251:54863, Nov  1 2007, 13:11:14) 
[GCC 4.1.2 (Gentoo 4.1.2 p1.0.2)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> from twisted.python.reflect import namedClass
>>> class Test(object):
...     pass
... 
>>> namedClass("__main__.Testz")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib64/python2.5/site-packages/twisted/python/reflect.py", line 344, in namedObject
    return getattr(module, classSplit[-1])
AttributeError: 'module' object has no attribute 'Testz'
>>> namedClass("__main__.Test")
<class '__main__.Test'>

What if the values of one of my parameters is a dictionary, do we want to be able to merge the defaults with what is specified in the config file? Does this work already?

This should already work as a result of the recursive nature of the _mergeData() function. I'll try and whip up a test case.

Changed 6 years ago by callan@…

Updated patch with test cases covering the possibility of dictionary based parameters

Changed 6 years ago by callan@…

Further dictionary test fixes

comment:13 Changed 4 years ago by wsanchez@…

  • Owner changed from callan@… to sagen@…
  • Milestone changed from CalendarServer-2.x to CalendarServer-3.x

comment:14 Changed 3 years ago by wsanchez@…

  • Milestone changed from CalendarServer-3.x to Later
Note: See TracTickets for help on using tickets.