Skip to content

Commit

Permalink
Fix the original deprecate_context_item function.
Browse files Browse the repository at this point in the history
  • Loading branch information
Ian Murray committed Aug 2, 2012
1 parent 95a38b5 commit 3ae477b
Showing 1 changed file with 5 additions and 29 deletions.
34 changes: 5 additions & 29 deletions ckan/lib/maintain.py
Expand Up @@ -62,34 +62,10 @@ def deprecate_context_item(item_name, message=''):
# prevent a circular import
from ckan.lib.base import c

class WrappedContextItem(object):
''' This is a fake object that calls the methods of the object
contained. '''
def __init__(self, obj, message):
self._ckan_obj = obj
self._ckan_message = message
def __getattribute__(self, name):
message = object.__getattribute__(self, '_ckan_message')
def get_item(self):
if self == c:

This comment has been minimized.

Copy link
@icmurray

icmurray Aug 9, 2012

Contributor

This should be self is c, not self == c

log.warning('c.%s has been deprecated. %s', item_name, message)
obj = object.__getattribute__(self, '_ckan_obj')
# hack to get the actual object when needed
if name == '_ckan_obj':
return obj
return getattr(obj, name)


# store the value in a fake object
setattr(c, item_name, WrappedContextItem(getattr(c, item_name), message))

# we need to store the origional __getattr__ and replace with our own one
if not hasattr(c.__class__, '__old_getattr__'):
def fake_attr(self, name):
obj = self.__class__.__dict__['__old_getattr__'](self, name)
if isinstance(obj, WrappedContextItem):
return obj._ckan_obj
else:
return obj
get_attr = getattr(c.__class__, '__getattr__')
setattr(c.__class__, '__old_getattr__', get_attr)
setattr(c.__class__, '__getattr__', fake_attr)
return getattr(self._current_obj(), item_name)

setattr(c.__class__, item_name, property(get_item))

2 comments on commit 3ae477b

@tobes
Copy link
Contributor

@tobes tobes commented on 3ae477b Aug 2, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@icmurray,

Thanks,

I'm a little worried with setattr(c.__class__, item_name, property(get_item)) is it thread safe?

To be honest I've no real comprehension of how c works I've look at some of the code but it was a bit too convoluted for my patience

@kindly
Copy link
Contributor

@kindly kindly commented on 3ae477b Aug 4, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setattr(c.__class__, item_name, property(get_item))

does not look thread safe to me.

I also do not like tobes WrappedContextItem as it seems unnecessary and has a risk of not supporting all types of objects.

I would monkey patch the context classes __getattr__ like in tobes solution (probably at startup) but add something on the context instance like

c.__depricated_properties__ = { 'facet': 'facet deprication message' , ...}

c.__depricated_properties__ would populated and/or created by this function.

The overridden __getattr__ would check to see if c.__depricated_properties__ exists and if the name was in there then throw the warning. Then it would call the original context classes __getattr__ .

The only downside of this is another hidden variable.

Please sign in to comment.