Repository: disqus/python-phabricator

Issue:
Conflict between Resource class attribute and new almanac interface endpoints
With [rPe502df509d5f](https://secure.phabricator.com/rPe502df509d5f8dd3106be50d7d83ac244ff96cb4), upstream phab implements almanac conduit endpoints: `almanc.interface.search` and `almanc.interface.edit`.

This now conflicts with the existing Resource class attribute `self.interface`. If you reproduce this against a Phab instance with the new `almanac.interface.*` conduit endpoints:
```
from phabricator import Phabricator
phab = Phabricator()
phab.almanac.interface.search
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'dict' object has no attribute 'search'
```
it throws an error because it is using the Resource class attribute `self.interface` instead of treating it like a normal method. This does not occur when the method name does not conflict with the Resource attributes:
```
phab.almanac.device.search
<phabricator.Resource object at 0x7f760699cf60>
```

The Resource class should not rely on certain keywords *not* becoming conduit methods.

Discussion:

Patch:
diff --git a/CHANGES b/CHANGES
index 2fd4c34..64c95a0 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+0.8.1
+
+* Fixed bug where built-in interface dict overwrote `interface` methods from Almanac (`interface.search`, `interface.edit`). The `Resource` attribute `interface` is renamed `_interface`.
+
 0.8.0
 
 * Switch to using requests
diff --git a/phabricator/__init__.py b/phabricator/__init__.py
index b01d167..6d53748 100644
--- a/phabricator/__init__.py
+++ b/phabricator/__init__.py
@@ -224,7 +224,7 @@ class Result(MutableMapping):
 class Resource(object):
     def __init__(self, api, interface=None, endpoint=None, method=None, nested=False):
         self.api = api
-        self.interface = interface or copy.deepcopy(parse_interfaces(INTERFACES))
+        self._interface = interface or copy.deepcopy(parse_interfaces(INTERFACES))
         self.endpoint = endpoint
         self.method = method
         self.nested = nested
@@ -232,7 +232,7 @@ class Resource(object):
     def __getattr__(self, attr):
         if attr in getattr(self, '__dict__'):
             return getattr(self, attr)
-        interface = self.interface
+        interface = self._interface
         if self.nested:
             attr = "%s.%s" % (self.endpoint, attr)
         submethod_exists = False
@@ -254,7 +254,7 @@ class Resource(object):
 
     def _request(self, **kwargs):
         # Check for missing variables
-        resource = self.interface
+        resource = self._interface
 
         def validate_kwarg(key, target):
             # Always allow list
@@ -391,4 +391,4 @@ class Phabricator(Resource):
 
         interfaces = query()
 
-        self.interface = parse_interfaces(interfaces)
+        self._interface = parse_interfaces(interfaces)
diff --git a/setup.py b/setup.py
index c7586bc..ec3f0e5 100644
--- a/setup.py
+++ b/setup.py
@@ -14,7 +14,7 @@ if sys.version_info[:2] <= (3, 3):
 
 setup(
     name='phabricator',
-    version='0.8.0',
+    version='0.8.1',
     author='Disqus',
     author_email='opensource@disqus.com',
     url='http://github.com/disqus/python-phabricator',


Test patch:
diff --git a/phabricator/tests/test_phabricator.py b/phabricator/tests/test_phabricator.py
index cad5a8e..e12b285 100644
--- a/phabricator/tests/test_phabricator.py
+++ b/phabricator/tests/test_phabricator.py
@@ -163,7 +163,7 @@ class PhabricatorTest(unittest.TestCase):
 
 
     def test_endpoint_shadowing(self):
-        shadowed_endpoints = [e for e in self.api.interface.keys() if e in self.api.__dict__]
+        shadowed_endpoints = [e for e in self.api._interface.keys() if e in self.api.__dict__]
         self.assertEqual(
             shadowed_endpoints,
             [],