Provide more specific exceptions if the CacheRegion is already configured

Issue #40 resolved
Anonymous created an issue

Ideally, CacheRegion should raise a dogpile.cache specific exception that can be handled if the region is already configured (and similarly if .backend is attempted to be accessed without the region being configured).

There should also be a method to determine if a Region is already configured (in a friendly manner).

Comments (5)

  1. Michael Bayer repo owner

    there's a couple of issues raised here so let me know your comments on this:

    from dogpile.cache import make_region
    
    reg = make_region()
    
    # check if region is configured...
    try:
        assert reg.backend is None
    except Exception as e:
        # OK, not friendly
        print(e)
    
    # check for now, but not friendly, OK.
    assert "backend" not in reg.__dict__
    
    # configure.
    reg.configure("dogpile.cache.memory")
    
    # check if region is configured.
    assert reg.backend is not None
    
    # this raises an exception as requested, but is Exception -
    # dogpile.cache-specific exception is important why?  only because no
    # easy way to check for backend exists? 
    reg.configure("dogpile.cache.memory")
    
  2. Morgan Fainberg

    As it stands, the code is workable, and it isn't a bear to deal with. However, a specific dogpile.cache exception would be better to handle than Exception, in the case that some non-configuration exception occurs, that way it would be possible to handle a second configuration attempt gracefully, but not handle all possible exceptions.

    Right now dogpile.cache simply raises Exception when it has already been configured.

    In general, raising more specific exceptions makes for handling those types of errors independently of other errors

    Example (more ideal) code:

    import sys
    
    from dogpile.cache import make_region
    from dogpile.cache.exception import AlreadyConfigured
    
    reg = make_region()
    try:
        reg.configure('dogpile.cache.memory')
    except  AlreadyConfigured:
        # TypeErrors wouldn't be caught here, if say a list was passed to configure()
        # this is also easier to read than inspecting the exception to see what type it is
        print 'Cache Region is already configured'
    except Exception as e:
        # handle all other exceptions by exiting
        sys.exit(1)
    

    It might also be useful to have an @Property .is_configured to reference instead of having to try/except to validate a configuration has been performed.

    If you don't get to it first, I'll see if I can propose a changeset to cover this type of change.

  3. Michael Bayer repo owner

    is_configured is great and feel free to implement AlreadyConfigured but I'd make it a subclass of DogpileException or something like that....can you locate other places we're raising Exception and fix those too ?

  4. Log in to comment