fix: Remove defaults for feature flag calls (#72)

Author: neilkakkar

CHANGELOG.md

@@ -1,3 +1,9 @@
+## 2.1.0 - 2022-08-11
+
+Changes:
+
+1. Feature flag defaults have been removed
+2. Setup logging only when debug mode is enabled.
 ## 2.0.1 - 2022-08-04
 
 - Make poll_interval configurable
@@ -9,7 +15,8 @@
 Breaking changes:
 
 1. The minimum version requirement for PostHog servers is now 1.38. If you're using PostHog Cloud, you satisfy this requirement automatically.
-2. Feature flag defaults apply only when there's an error fetching feature flag results. Earlier, if the default was set to `True`, even if a flag resolved to `False`, the default would override this.
+2. Feature flag defaults apply only when there's an error fetching feature flag results. Earlier, if the default was set to `True`, even if a flag resolved to `False`, the default would override this. 
+**Note: These are removed in 2.0.2**
 3. Feature flag remote evaluation doesn't require a personal API key.
 
 New Changes:

posthog/__init__.py

@@ -235,7 +235,6 @@ def alias(
 def feature_enabled(
     key,  # type: str,
     distinct_id,  # type: str,
-    default=False,  # type: bool
     groups={},  # type: dict
     person_properties={},  # type: dict
     group_properties={},  # type: dict
@@ -260,7 +259,6 @@ def feature_enabled(
         "feature_enabled",
         key=key,
         distinct_id=distinct_id,
-        default=default,
         groups=groups,
         person_properties=person_properties,
         group_properties=group_properties,
@@ -272,7 +270,6 @@ def feature_enabled(
 def get_feature_flag(
     key,  # type: str,
     distinct_id,  # type: str,
-    default=False,  # type: bool
     groups={},  # type: dict
     person_properties={},  # type: dict
     group_properties={},  # type: dict
@@ -305,7 +302,6 @@ def get_feature_flag(
         "get_feature_flag",
         key=key,
         distinct_id=distinct_id,
-        default=default,
         groups=groups,
         person_properties=person_properties,
         group_properties=group_properties,

posthog/client.py

@@ -23,10 +23,6 @@
 ID_TYPES = (numbers.Number, string_types, UUID)
 MAX_DICT_SIZE = 50_000
 
-# Ensures that debug level messages are logged when debug mode is on.
-# Otherwise, defaults to WARNING level. See https://docs.python.org/3/howto/logging.html#what-happens-if-no-configuration-is-provided
-logging.basicConfig()
-
 
 class Client(object):
     """Create a new PostHog client."""
@@ -76,6 +72,9 @@ def __init__(
         # personal_api_key: This should be a generated Personal API Key, private
         self.personal_api_key = personal_api_key
         if debug:
+            # Ensures that debug level messages are logged when debug mode is on.
+            # Otherwise, defaults to WARNING level. See https://docs.python.org/3/howto/logging.html#what-happens-if-no-configuration-is-provided
+            logging.basicConfig()
             self.log.setLevel(logging.DEBUG)
         else:
             self.log.setLevel(logging.WARNING)
@@ -432,32 +431,31 @@ def feature_enabled(
         self,
         key,
         distinct_id,
-        default=False,
         *,
         groups={},
         person_properties={},
         group_properties={},
         only_evaluate_locally=False,
         send_feature_flag_events=True,
     ):
-        return bool(
-            self.get_feature_flag(
-                key,
-                distinct_id,
-                default,
-                groups=groups,
-                person_properties=person_properties,
-                group_properties=group_properties,
-                only_evaluate_locally=only_evaluate_locally,
-                send_feature_flag_events=send_feature_flag_events,
-            )
+        response = self.get_feature_flag(
+            key,
+            distinct_id,
+            groups=groups,
+            person_properties=person_properties,
+            group_properties=group_properties,
+            only_evaluate_locally=only_evaluate_locally,
+            send_feature_flag_events=send_feature_flag_events,
         )
 
+        if response is None:
+            return None
+        return bool(response)
+
     def get_feature_flag(
         self,
         key,
         distinct_id,
-        default=False,
         *,
         groups={},
         person_properties={},
@@ -500,10 +498,11 @@ def get_feature_flag(
                     distinct_id, groups=groups, person_properties=person_properties, group_properties=group_properties
                 )
                 response = feature_flags.get(key)
+                if response is None:
+                    response = False
                 self.log.debug(f"Successfully computed flag remotely: #{key} -> #{response}")
             except Exception as e:
                 self.log.exception(f"[FEATURE FLAGS] Unable to get flag remotely: {e}")
-                response = default
 
         feature_flag_reported_key = f"{key}_{str(response)}"
         if (

posthog/test/test_feature_flags.py

@@ -385,7 +385,7 @@ def test_feature_flags_dont_fallback_to_decide_when_only_local_evaluation_is_tru
 
         feature_flag_match = client.feature_enabled("beta-feature", "some-distinct-id", only_evaluate_locally=True)
 
-        self.assertEqual(feature_flag_match, False)
+        self.assertEqual(feature_flag_match, None)
         self.assertEqual(patch_decide.call_count, 0)
 
         # beta-feature2 should fallback to decide because region property not given with call
@@ -394,13 +394,13 @@ def test_feature_flags_dont_fallback_to_decide_when_only_local_evaluation_is_tru
         self.assertEqual(feature_flag_match, None)
 
         feature_flag_match = client.feature_enabled("beta-feature2", "some-distinct-id", only_evaluate_locally=True)
-        self.assertEqual(feature_flag_match, False)
+        self.assertEqual(feature_flag_match, None)
 
         self.assertEqual(patch_decide.call_count, 0)
 
     @mock.patch("posthog.client.decide")
     @mock.patch("posthog.client.get")
-    def test_feature_flag_defaults_dont_hinder_regular_evaluation(self, patch_get, patch_decide):
+    def test_feature_flag_never_returns_undefined_during_regular_evaluation(self, patch_get, patch_decide):
         patch_decide.return_value = {"featureFlags": {}}
         client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
         client.feature_flags = [
@@ -422,28 +422,28 @@ def test_feature_flag_defaults_dont_hinder_regular_evaluation(self, patch_get, p
         ]
 
         # beta-feature resolves to False, so no matter the default, stays False
-        self.assertFalse(client.get_feature_flag("beta-feature", "some-distinct-id", default=True))
-        self.assertFalse(client.get_feature_flag("beta-feature", "some-distinct-id", default=False))
+        self.assertFalse(client.get_feature_flag("beta-feature", "some-distinct-id"))
+        self.assertFalse(client.feature_enabled("beta-feature", "some-distinct-id"))
 
         # beta-feature2 falls back to decide, and whatever decide returns is the value
-        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id", default=False))
+        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 1)
 
-        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id", default=True))
+        self.assertFalse(client.feature_enabled("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 2)
 
     @mock.patch("posthog.client.decide")
     @mock.patch("posthog.client.get")
-    def test_feature_flag_defaults_come_into_play_only_when_decide_errors_out(self, patch_get, patch_decide):
+    def test_feature_flag_return_none_when_decide_errors_out(self, patch_get, patch_decide):
         patch_decide.side_effect = APIError(400, "Decide error")
         client = Client(FAKE_TEST_API_KEY, personal_api_key=FAKE_TEST_API_KEY)
         client.feature_flags = []
 
-        # beta-feature2 falls back to decide, which on error falls back to default
-        self.assertFalse(client.get_feature_flag("beta-feature2", "some-distinct-id", default=False))
+        # beta-feature2 falls back to decide, which on error returns None
+        self.assertIsNone(client.get_feature_flag("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 1)
 
-        self.assertTrue(client.get_feature_flag("beta-feature2", "some-distinct-id", default=True))
+        self.assertIsNone(client.feature_enabled("beta-feature2", "some-distinct-id"))
         self.assertEqual(patch_decide.call_count, 2)
 
     @mock.patch("posthog.client.decide")
@@ -943,7 +943,7 @@ def test_feature_enabled_doesnt_exist(self, patch_decide, patch_poll):
         self.assertFalse(client.feature_enabled("doesnt-exist", "distinct_id"))
 
         patch_decide.side_effect = APIError(401, "decide error")
-        self.assertTrue(client.feature_enabled("doesnt-exist", "distinct_id", True))
+        self.assertIsNone(client.feature_enabled("doesnt-exist", "distinct_id"))
 
     @mock.patch("posthog.client.Poller")
     @mock.patch("posthog.client.decide")

posthog/version.py

@@ -1,4 +1,4 @@
-VERSION = "2.0.1"
+VERSION = "2.1.0"
 
 if __name__ == "__main__":
     print(VERSION, end="")