Responding to Matt's feedback.

optimizely/config_manager.py

@@ -71,9 +71,9 @@ def __init__(self,
         super(StaticConfigManager, self).__init__(logger=logger, error_handler=error_handler)
         self._config = None
         self.validate_schema = not skip_json_validation
-        self.set_config(datafile)
+        self._set_config(datafile)
 
-    def set_config(self, datafile):
+    def _set_config(self, datafile):
         """ Looks up and sets datafile and config based on response body.
 
          Args:
@@ -103,9 +103,17 @@ def set_config(self, datafile):
                 self.error_handler.handle_error(error_to_handle)
                 return
 
+        previous_revision = self._config.get_revision() if self._config else None
+
+        if previous_revision == config.get_revision():
+            return
+
         # TODO(ali): Add notification listener.
         self._config = config
-        self.logger.debug('Received new datafile and updated config.')
+        self.logger.debug(
+            'Received new datafile and updated config. '
+            'Old revision number: {}. New revision number: {}.'.format(previous_revision, config.get_revision())
+        )
 
     def get_config(self):
         """ Returns instance of ProjectConfig.
@@ -156,7 +164,7 @@ def __init__(self,
         self._polling_thread = threading.Thread(target=self._run)
         self._polling_thread.setDaemon(True)
         if datafile:
-            self.set_config(datafile)
+            self._set_config(datafile)
         self._polling_thread.start()
 
     @staticmethod
@@ -233,7 +241,7 @@ def _handle_response(self, response):
             return
 
         self.set_last_modified(response.headers)
-        self.set_config(response.content)
+        self._set_config(response.content)
 
     def fetch_datafile(self):
         """ Fetch datafile and set ProjectConfig. """

tests/test_config_manager.py

@@ -31,8 +31,25 @@ def test_set_config__success(self):
         project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile,
                                                                     logger=mock_logger)
 
-        project_config_manager.set_config(test_datafile)
-        mock_logger.debug.assert_called_with('Received new datafile and updated config.')
+        project_config_manager._set_config(test_datafile)
+        mock_logger.debug.assert_called_with('Received new datafile and updated config. '
+                                             'Old revision number: None. New revision number: 1.')
+
+    def test_set_config__twice(self):
+        """ Test calling set_config twice with same content to ensure config is not updated. """
+        test_datafile = json.dumps(self.config_dict_with_features)
+        mock_logger = mock.Mock()
+        project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile,
+                                                                    logger=mock_logger)
+
+        project_config_manager._set_config(test_datafile)
+        mock_logger.debug.assert_called_with('Received new datafile and updated config. '
+                                             'Old revision number: None. New revision number: 1.')
+        self.assertEqual(1, mock_logger.debug.call_count)
+
+        # Call set config again and confirm that no new log message denoting config update is there
+        project_config_manager._set_config(test_datafile)
+        self.assertEqual(1, mock_logger.debug.call_count)
 
     def test_set_config__schema_validation(self):
         """ Test set_config calls or does not call schema validation based on skip_json_validation value. """
@@ -70,7 +87,7 @@ def test_set_config__unsupported_datafile_version(self):
         test_datafile = json.dumps(invalid_version_datafile)
 
         # Call set_config with datafile having invalid version
-        project_config_manager.set_config(test_datafile)
+        project_config_manager._set_config(test_datafile)
         mock_logger.error.assert_called_once_with('This version of the Python SDK does not support '
                                                   'the given datafile version: "invalid_version".')
 
@@ -84,7 +101,7 @@ def test_set_config__invalid_datafile(self):
                                                                     logger=mock_logger)
 
         # Call set_config with invalid content
-        project_config_manager.set_config('invalid_datafile')
+        project_config_manager._set_config('invalid_datafile')
         mock_logger.error.assert_called_once_with('Provided "datafile" is in an invalid format.')
 
     def test_get_config(self):

tests/test_decision_service.py

@@ -855,7 +855,8 @@ def test_get_variation_for_feature__returns_none_for_invalid_group_id(self):
         self.decision_service.get_variation_for_feature(self.project_config, feature, 'test_user')
       )
     mock_decision_service_logging.error.assert_called_once_with(
-      enums.Errors.INVALID_GROUP_ID_ERROR.format('_get_variation_for_feature')
+      enums.Errors.INVALID_GROUP_ID.format('_get_variation_for_feature')
+    )
 
   def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_not_associated_with_feature(self):
     """ Test that if a user is in the mutex group but the experiment is

tests/test_optimizely.py

@@ -195,7 +195,7 @@ def test_init__datafile_only(self):
   def test_init__sdk_key_only(self):
     """ Test that if only sdk_key is provided then PollingConfigManager is used. """
 
-    with mock.patch('optimizely.config_manager.PollingConfigManager.set_config'), \
+    with mock.patch('optimizely.config_manager.PollingConfigManager._set_config'), \
       mock.patch('threading.Thread.start'):
       opt_obj = optimizely.Optimizely(sdk_key='test_sdk_key')
 
@@ -204,7 +204,7 @@ def test_init__sdk_key_only(self):
   def test_init__sdk_key_and_datafile(self):
     """ Test that if both sdk_key and datafile is provided then PollingConfigManager is used. """
 
-    with mock.patch('optimizely.config_manager.PollingConfigManager.set_config'), \
+    with mock.patch('optimizely.config_manager.PollingConfigManager._set_config'), \
       mock.patch('threading.Thread.start'):
       opt_obj = optimizely.Optimizely(datafile=json.dumps(self.config_dict), sdk_key='test_sdk_key')