Provide more specific exceptions if the CacheRegion is already configured

[sqlalchemy-bot]

Migrated issue, originally created by Anonymous

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).

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

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

#!python

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")

[sqlalchemy-bot]

Morgan Fainberg () wrote:

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:

#!python

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.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

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 ?

[sqlalchemy-bot]

Morgan Fainberg () wrote:

I will definitely do the exception work as well. Once I have a few spare cycles to work on this I'll get you a pull request with the proposed changes.

[sqlalchemy-bot]

Michael Bayer (zzzeek) wrote:

merge is 00dbd18 and a few tweaks in 9f186db thanks!

[sqlalchemy-bot]

Changes by Michael Bayer (zzzeek):

• changed status to closed