Addressing PR comments and fixing async flow during fetch call

Author: Pijush Chakraborty

firebase_admin/remote_config.py

@@ -16,6 +16,7 @@
 This module has required APIs for the clients to use Firebase Remote Config with python.
 """
 
+import asyncio
 import json
 from typing import Any, Dict, Optional
 import requests
@@ -25,7 +26,7 @@
 _REMOTE_CONFIG_ATTRIBUTE = '_remoteconfig'
 
 class ServerTemplateData:
-    """Represents a Server Template Data class."""
+    """Parses, validates and encapsulates template data and metadata."""
     def __init__(self, etag, template_data):
         """Initializes a new ServerTemplateData instance.
 
@@ -78,7 +79,7 @@ def conditions(self):
 
 
 class ServerTemplate:
-    """Represents a Server Template with implementations for loading and evaluting the tempalte."""
+    """Represents a Server Template with implementations for loading and evaluting the template."""
     def __init__(self, app: App = None, default_config: Optional[Dict[str, str]] = None):
         """Initializes a ServerTemplate instance.
 
@@ -93,14 +94,18 @@ def __init__(self, app: App = None, default_config: Optional[Dict[str, str]] = N
         # This gets set when the template is
         # fetched from RC servers via the load API, or via the set API.
         self._cache = None
+        self._stringified_default_config: Dict[str,str] = {}
+
+        # RC stores all remote values as string, but it's more intuitive
+        # to declare default values with specific types, so this converts
+        # the external declaration to an internal string representation.
         if default_config is not None:
-            self._stringified_default_config = json.dumps(default_config)
-        else:
-            self._stringified_default_config = None
+            for key in default_config:
+                self._stringified_default_config[key] = str(default_config[key])
 
     async def load(self):
         """Fetches the server template and caches the data."""
-        self._cache = self._rc_service.get_server_template()
+        self._cache = await self._rc_service.get_server_template()
 
     def evaluate(self):
         # Logic to process the cached template into a ServerConfig here.
@@ -157,21 +162,23 @@ def __init__(self, app):
                                                    base_url=remote_config_base_url,
                                                    headers=rc_headers, timeout=timeout)
 
-    def get_server_template(self):
+    async def get_server_template(self):
         """Requests for a server template and converts the response to an instance of
         ServerTemplateData for storing the template parameters and conditions."""
-        url_prefix = self._get_url_prefix()
         try:
-            headers, response_json = self._client.headers_and_body(
-                'get', url=url_prefix+'/namespaces/firebase-server/serverRemoteConfig')
+            loop = asyncio.get_event_loop()
+            headers, template_data = await loop.run_in_executor(None, 
+                                                                self._client.headers_and_body, 
+                                                                'get', self._get_url())
         except requests.exceptions.RequestException as error:
             raise self._handle_remote_config_error(error)
         else:
-            return ServerTemplateData(headers.get('etag'), response_json)
+            return ServerTemplateData(headers.get('etag'), template_data)
 
-    def _get_url_prefix(self):
+    def _get_url(self):
         """Returns project prefix for url, in the format of /v1/projects/${projectId}"""
-        return "/v1/projects/{0}".format(self._project_id)
+        return "/v1/projects/{0}/namespaces/firebase-server/serverRemoteConfig".format(
+            self._project_id)
 
     @classmethod
     def _handle_remote_config_error(cls, error: Any):

tests/test_functions.py

@@ -209,7 +209,7 @@ def test_task_options(self, task_opts_params):
 
         schedule_time = datetime.fromisoformat(task['schedule_time'][:-1])
         delta = abs(schedule_time - _SCHEDULE_TIME)
-        assert delta <= timedelta(seconds=30)
+        assert delta <= timedelta(seconds=15)
 
         assert task['dispatch_deadline'] == '200s'
         assert task['http_request']['headers']['x-test-header'] == 'test-header-value'

tests/test_remote_config.py

@@ -38,7 +38,8 @@ def send(self, request, **kwargs):
         return resp
 
 
-class TestRemoteConfigServiceClient:
+class TestRemoteConfigService:
+    """Tests methods on _RemoteConfigService"""
     @classmethod
     def setup_class(cls):
         cred = testutils.MockCredential()
@@ -48,7 +49,8 @@ def setup_class(cls):
     def teardown_class(cls):
         testutils.cleanup_apps()
 
-    def test_rc_instance_get_server_template(self):
+    @pytest.mark.asyncio
+    async def test_rc_instance_get_server_template(self):
         recorder = []
         response = json.dumps({
             'parameters': {
@@ -64,13 +66,14 @@ def test_rc_instance_get_server_template(self):
             'https://firebaseremoteconfig.googleapis.com',
             MockAdapter(response, 200, recorder))
 
-        template = rc_instance.get_server_template()
+        template = await rc_instance.get_server_template()
 
         assert template.parameters == dict(test_key="test_value")
         assert str(template.version) == 'test'
         assert str(template.etag) == 'etag'
 
-    def test_rc_instance_get_server_template_empty_params(self):
+    @pytest.mark.asyncio
+    async def test_rc_instance_get_server_template_empty_params(self):
         recorder = []
         response = json.dumps({
             'conditions': [],
@@ -83,14 +86,15 @@ def test_rc_instance_get_server_template_empty_params(self):
             'https://firebaseremoteconfig.googleapis.com',
             MockAdapter(response, 200, recorder))
 
-        template = rc_instance.get_server_template()
+        template = await rc_instance.get_server_template()
 
         assert template.parameters == {}
         assert str(template.version) == 'test'
         assert str(template.etag) == 'etag'
 
 
-class TestRemoteConfigService:
+class TestRemoteConfigModule:
+    """Tests methods on firebase_admin.remote_config"""
     @classmethod
     def setup_class(cls):
         cred = testutils.MockCredential()
@@ -112,6 +116,7 @@ def test_init_server_template(self):
 
         template = remote_config.init_server_template(
             app=app,
+            default_config={'default_test': 'default_value'},
             template_data=ServerTemplateData('etag', template_data)
         )