Fix is_same_callback() testing for python3.8

Python3.8 onwards, comparision of bound methods is
changed. It no longer decide the bound method's equality based
on their bounded objects equality instead it checks the identity
of their '__self__'.

Details about this behavior change in python 3.8
- https://bugs.python.org/issue1617161
- python-dev discussion: https://mail.python.org/pipermail/python-dev/2018-June/153959.html

So python3.8 onwards, 'strict' arg has no meaning. For backward compatibility
for python <3.8, we can keep the 'strict' arg but with deprecation warning.

Also modify the is_same_callback() unit tests to verify the 'strict'
arg based on python version.

Change-Id: I3e6d9bbceccacddd0e1a514bbe5d0722a3408ecb
Closes-Bug: #1841072

2 files changed, 43 insertions, 7 deletions

diff --git a/oslo_utils/reflection.py b/oslo_utils/reflection.py
index f1801b8..c6b360e 100644
--- a/oslo_utils/reflection.py
+++ b/oslo_utils/reflection.py
@@ -21,6 +21,7 @@ Reflection module.
 """
 
 import inspect
+import logging
 import types
 
 import six
@@ -35,6 +36,10 @@ except AttributeError:
 # others)...
 _BUILTIN_MODULES = ('builtins', '__builtin__', '__builtins__', 'exceptions')
 
+
+LOG = logging.getLogger(__name__)
+
+
 if six.PY3:
     Parameter = inspect.Parameter
     Signature = inspect.Signature
@@ -164,19 +169,39 @@ def get_method_self(method):
 
 
 def is_same_callback(callback1, callback2, strict=True):
-    """Returns if the two callbacks are the same."""
+    """Returns if the two callbacks are the same.
+
+    'strict' arg has no meaning for python 3.8 onwards and will
+    always return the equality of both callback based on 'self'
+    comparison only.
+    """
     if callback1 is callback2:
         # This happens when plain methods are given (or static/non-bound
         # methods).
         return True
     if callback1 == callback2:
+        # NOTE(gmann): python3.8 onward, comparison of bound methods is
+        # changed. It no longer decide the bound method's equality based
+        # on their bounded objects equality instead it checks the identity
+        # of their '__self__'. So python3.8 onward, two different bound
+        # methods are no longer equal even __eq__ method return True.
+        # Or in other term, 'strict' arg has no meaning from python 3.8
+        # onwards above if condition never satisfy if both callback are
+        # bounded to two different objects.
+        # For backward compatibility for python <3.8, we can keep the 'strict'
+        # arg and the below code of comparing 'self' and once minimum
+        # supported python version is 3.8 we can remove both because python
+        # 3.8 onward == operator itself checks identity of 'self'.
+        # Ref bug: https://bugs.launchpad.net/oslo.utils/+bug/1841072
         if not strict:
+            LOG.warning('"strict" arg is deprecated because it no '
+                        'longer work for python 3.8 onwards')
             return True
-        # Two bound methods are equal if functions themselves are equal and
-        # objects they are applied to are equal. This means that a bound
-        # method could be the same bound method on another object if the
-        # objects have __eq__ methods that return true (when in fact it is a
-        # different bound method). Python u so crazy!
+        # Until python 3.7, two bound methods are equal if functions
+        # themselves are equal and objects they are applied to are equal.
+        # This means that a bound method could be the same bound method on
+        # another object if the objects have __eq__ methods that return true
+        # (when in fact it is a different bound method). Python u so crazy!
         try:
             self1 = six.get_method_self(callback1)
             self2 = six.get_method_self(callback2)
diff --git a/oslo_utils/tests/test_reflection.py b/oslo_utils/tests/test_reflection.py
index 34384f7..84ba607 100644
--- a/oslo_utils/tests/test_reflection.py
+++ b/oslo_utils/tests/test_reflection.py
@@ -14,6 +14,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import sys
+
 from oslotest import base as test_base
 import six
 import testtools
@@ -153,7 +155,16 @@ class CallbackEqualityTest(test_base.BaseTestCase):
         c = A()
 
         self.assertFalse(reflection.is_same_callback(b.b, c.b))
-        self.assertTrue(reflection.is_same_callback(b.b, c.b, strict=False))
+        # NOTE(gmann): python3.8 onwards, comparision of bound methods is
+        # changed and 'strict' arg has no meaning.
+        # Ref bug: https://bugs.launchpad.net/oslo.utils/+bug/1841072
+        if sys.version_info < (3, 8):
+            self.assertTrue(reflection.is_same_callback(b.b, c.b,
+                            strict=False))
+        else:
+            self.assertFalse(reflection.is_same_callback(b.b, c.b,
+                             strict=False))
+            self.assertTrue(reflection.is_same_callback(b.b, b.b))
 
 
 class BoundMethodTest(test_base.BaseTestCase):