Repository: CartoDB/cartoframes
Pull request #1728 (2021-03-26 08:40:23)

Improve performance in subscriptions
- Use "only_products" param for DO API
- Fetch only subscriptions when accessing datasets/geographies


Patch:
diff --git a/cartoframes/data/observatory/catalog/catalog.py b/cartoframes/data/observatory/catalog/catalog.py
index 050b868f..693df4b5 100644
--- a/cartoframes/data/observatory/catalog/catalog.py
+++ b/cartoframes/data/observatory/catalog/catalog.py
@@ -286,13 +286,9 @@ class Catalog:
             CatalogError: if there's a problem when connecting to the catalog or no datasets are found.
 
         """
-        _no_filters = {}
         _credentials = get_credentials(credentials)
 
-        return Subscriptions(
-            Dataset.get_all(_no_filters, _credentials),
-            Geography.get_all(_no_filters, _credentials)
-        )
+        return Subscriptions(_credentials)
 
     def datasets_filter(self, filter_dataset):
         """Get all the datasets in the Catalog filtered
diff --git a/cartoframes/data/observatory/catalog/repository/dataset_repo.py b/cartoframes/data/observatory/catalog/repository/dataset_repo.py
index cc9f906a..4bd26ade 100644
--- a/cartoframes/data/observatory/catalog/repository/dataset_repo.py
+++ b/cartoframes/data/observatory/catalog/repository/dataset_repo.py
@@ -25,7 +25,6 @@ class DatasetRepository(EntityRepository):
 
         # Using user credentials to fetch entities
         self.client.set_user_credentials(credentials)
-        filters = self.client.get_entities_filters(filters)
         entities = self._get_filtered_entities(filters)
         self.client.reset_user_credentials()
         return entities
diff --git a/cartoframes/data/observatory/catalog/repository/geography_repo.py b/cartoframes/data/observatory/catalog/repository/geography_repo.py
index 19d8f652..d23fc922 100644
--- a/cartoframes/data/observatory/catalog/repository/geography_repo.py
+++ b/cartoframes/data/observatory/catalog/repository/geography_repo.py
@@ -28,7 +28,6 @@ class GeographyRepository(EntityRepository):
 
         # Using user credentials to fetch entities
         self.client.set_user_credentials(credentials)
-        filters = self.client.get_entities_filters(filters)
         entities = self._get_filtered_entities(filters)
         self.client.reset_user_credentials()
         return entities
diff --git a/cartoframes/data/observatory/catalog/repository/repo_client.py b/cartoframes/data/observatory/catalog/repository/repo_client.py
index 6ef42d99..74e50b63 100644
--- a/cartoframes/data/observatory/catalog/repository/repo_client.py
+++ b/cartoframes/data/observatory/catalog/repository/repo_client.py
@@ -73,13 +73,6 @@ class RepoClient:
             entity = 'datasets/{0}/variables_groups'.format(filters.pop('dataset'))
             return self._fetch_entity(entity, filters)
 
-    def get_entities_filters(self, filters=None):
-        self.set_external_credentials()
-        if filters is not None:
-            filters['filter_catalog'] = False
-        entities = self._fetch_entity('entities', filters)
-        return {'id': [result['id'] for result in entities['results']]}
-
     def _get_filter_id(self, filters, use_slug=False):
         if isinstance(filters, dict):
             filter_id = filters.get('id')
@@ -101,5 +94,7 @@ class RepoClient:
             return self._fetch_entity('{0}/{1}'.format(entity, filter_id))
 
     def _fetch_entity(self, entity, filters=None):
+        filters = filters or {}
+        filters['only_products'] = True
         do_dataset = self._user_do_dataset or self._external_do_dataset or self._default_do_dataset
         return do_dataset.metadata(entity, filters)
diff --git a/cartoframes/data/observatory/catalog/subscriptions.py b/cartoframes/data/observatory/catalog/subscriptions.py
index ebf09830..82a7e350 100644
--- a/cartoframes/data/observatory/catalog/subscriptions.py
+++ b/cartoframes/data/observatory/catalog/subscriptions.py
@@ -10,14 +10,16 @@ class Subscriptions:
     require a subscription.
 
     """
-    def __init__(self, datasets, geographies):
-        self._subscriptions_datasets = datasets
-        self._subscriptions_geographies = geographies
+    def __init__(self, credentials):
+        self._credentials = credentials
+        self._no_filters = {}
+        self._datasets = None
+        self._geographies = None
 
     def __repr__(self):
         return 'Datasets: {0}\nGeographies: {1}'.format(
-            self._subscriptions_datasets,
-            self._subscriptions_geographies
+            self.datasets,
+            self.geographies
         )
 
     @property
@@ -28,7 +30,10 @@ class Subscriptions:
             CatalogError: if there's a problem when connecting to the catalog.
 
         """
-        return self._subscriptions_datasets
+        if self._datasets is None:
+            from .dataset import Dataset
+            self._datasets = Dataset.get_all(self._no_filters, self._credentials)
+        return self._datasets
 
     @property
     def geographies(self):
@@ -38,7 +43,10 @@ class Subscriptions:
             CatalogError: if there's a problem when connecting to the catalog.
 
         """
-        return self._subscriptions_geographies
+        if self._geographies is None:
+            from .geography import Geography
+            self._geographies = Geography.get_all(self._no_filters, self._credentials)
+        return self._geographies
 
 
 def get_subscription_ids(credentials, stype=None):


Test patch:
diff --git a/tests/unit/data/observatory/catalog/repository/test_dataset_repo.py b/tests/unit/data/observatory/catalog/repository/test_dataset_repo.py
index cc92d742..20471a21 100644
--- a/tests/unit/data/observatory/catalog/repository/test_dataset_repo.py
+++ b/tests/unit/data/observatory/catalog/repository/test_dataset_repo.py
@@ -18,11 +18,9 @@ from ..examples import test_dataset1, test_datasets, db_dataset1, db_dataset2
 class TestDatasetRepo(object):
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_all(self, mocked_filters, mocked_repo):
+    def test_get_all(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_dataset1, db_dataset2]
-        mocked_filters.side_effect = lambda f: f
         repo = DatasetRepository()
 
         # When
@@ -34,12 +32,10 @@ class TestDatasetRepo(object):
         assert datasets == test_datasets
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
     @patch('cartoframes.data.observatory.catalog.repository.entity_repo.get_subscription_ids')
-    def test_get_all_credentials(self, mocked_get_subscription_ids, mocked_filters, mocked_get_datasets):
+    def test_get_all_credentials(self, mocked_get_subscription_ids, mocked_get_datasets):
         # Given
         mocked_get_subscription_ids.return_value = [db_dataset1['id'], db_dataset2['id']]
-        mocked_filters.side_effect = lambda f: f
         mocked_get_datasets.return_value = [db_dataset1, db_dataset2]
         credentials = Credentials('user', '1234')
         repo = DatasetRepository()
@@ -53,11 +49,9 @@ class TestDatasetRepo(object):
         assert datasets == test_datasets
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_all_when_empty(self, mocked_filters, mocked_repo):
+    def test_get_all_when_empty(self, mocked_repo):
         # Given
         mocked_repo.return_value = []
-        mocked_filters.side_effect = lambda f: f
         repo = DatasetRepository()
 
         # When
@@ -68,11 +62,9 @@ class TestDatasetRepo(object):
         assert datasets == []
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_all_only_uses_allowed_filters(self, mocked_filters, mocked_repo):
+    def test_get_all_only_uses_allowed_filters(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_dataset1, db_dataset2]
-        mocked_filters.side_effect = lambda f: f
         repo = DatasetRepository()
         filters = {
             COUNTRY_FILTER: 'usa',
@@ -98,11 +90,9 @@ class TestDatasetRepo(object):
         assert datasets == test_datasets
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_id(self, mocked_filters, mocked_repo):
+    def test_get_by_id(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_dataset1]
-        mocked_filters.side_effect = lambda f: f
         requested_id = db_dataset1['id']
         repo = DatasetRepository()
 
@@ -114,11 +104,9 @@ class TestDatasetRepo(object):
         assert dataset == test_dataset1
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_id_unknown_fails(self, mocked_filters, mocked_repo):
+    def test_get_by_id_unknown_fails(self, mocked_repo):
         # Given
         mocked_repo.return_value = []
-        mocked_filters.side_effect = lambda f: f
         requested_id = 'unknown_id'
         repo = DatasetRepository()
 
@@ -127,11 +115,9 @@ class TestDatasetRepo(object):
             repo.get_by_id(requested_id)
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_slug(self, mocked_filters, mocked_repo):
+    def test_get_by_slug(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_dataset1]
-        mocked_filters.side_effect = lambda f: f
         requested_slug = db_dataset1['slug']
         repo = DatasetRepository()
 
@@ -143,11 +129,9 @@ class TestDatasetRepo(object):
         assert dataset == test_dataset1
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_id_list(self, mocked_filters, mocked_repo):
+    def test_get_by_id_list(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_dataset1, db_dataset2]
-        mocked_filters.side_effect = lambda f: f
         repo = DatasetRepository()
 
         # When
@@ -159,11 +143,9 @@ class TestDatasetRepo(object):
         assert datasets == test_datasets
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_slug_list(self, mocked_filters, mocked_repo):
+    def test_get_by_slug_list(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_dataset1, db_dataset2]
-        mocked_filters.side_effect = lambda f: f
         repo = DatasetRepository()
 
         # When
@@ -175,11 +157,9 @@ class TestDatasetRepo(object):
         assert datasets == test_datasets
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_slug_and_id_list(self, mocked_filters, mocked_repo):
+    def test_get_by_slug_and_id_list(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_dataset1, db_dataset2]
-        mocked_filters.side_effect = lambda f: f
         repo = DatasetRepository()
 
         # When
@@ -191,11 +171,9 @@ class TestDatasetRepo(object):
         assert datasets == test_datasets
 
     @patch.object(RepoClient, 'get_datasets')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_missing_fields_are_mapped_as_None(self, mocked_filters, mocked_repo):
+    def test_missing_fields_are_mapped_as_None(self, mocked_repo):
         # Given
         mocked_repo.return_value = [{'id': 'dataset1'}]
-        mocked_filters.side_effect = lambda f: f
         repo = DatasetRepository()
 
         expected_datasets = CatalogList([Dataset({
diff --git a/tests/unit/data/observatory/catalog/repository/test_geography_repo.py b/tests/unit/data/observatory/catalog/repository/test_geography_repo.py
index 2a4efc61..5a1ed001 100644
--- a/tests/unit/data/observatory/catalog/repository/test_geography_repo.py
+++ b/tests/unit/data/observatory/catalog/repository/test_geography_repo.py
@@ -18,11 +18,9 @@ from ..examples import test_geography1, test_geographies, db_geography1, db_geog
 class TestGeographyRepo(object):
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_all(self, mocked_filters, mocked_repo):
+    def test_get_all(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1, db_geography2]
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
 
         # When
@@ -34,12 +32,10 @@ class TestGeographyRepo(object):
         assert geographies == test_geographies
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
     @patch('cartoframes.data.observatory.catalog.repository.entity_repo.get_subscription_ids')
-    def test_get_all_credentials(self, mocked_get_subscription_ids, mocked_filters, mocked_get_geographies):
+    def test_get_all_credentials(self, mocked_get_subscription_ids, mocked_get_geographies):
         # Given
         mocked_get_subscription_ids.return_value = [db_geography1['id'], db_geography2['id']]
-        mocked_filters.side_effect = lambda f: f
         mocked_get_geographies.return_value = [db_geography1, db_geography2]
         credentials = Credentials('user', '1234')
         repo = GeographyRepository()
@@ -53,11 +49,9 @@ class TestGeographyRepo(object):
         assert geographies == test_geographies
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_all_when_empty(self, mocked_filters, mocked_repo):
+    def test_get_all_when_empty(self, mocked_repo):
         # Given
         mocked_repo.return_value = []
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
 
         # When
@@ -68,11 +62,9 @@ class TestGeographyRepo(object):
         assert geographies == []
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_all_only_uses_allowed_filters(self, mocked_filters, mocked_repo):
+    def test_get_all_only_uses_allowed_filters(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1, db_geography2]
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
         filters = {
             COUNTRY_FILTER: 'usa',
@@ -97,11 +89,9 @@ class TestGeographyRepo(object):
         assert geographies == test_geographies
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_id(self, mocked_filters, mocked_repo):
+    def test_get_by_id(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1]
-        mocked_filters.side_effect = lambda f: f
         requested_id = db_geography1['id']
         repo = GeographyRepository()
 
@@ -114,11 +104,9 @@ class TestGeographyRepo(object):
         assert geography == test_geography1
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_id_unknown_fails(self, mocked_filters, mocked_repo):
+    def test_get_by_id_unknown_fails(self, mocked_repo):
         # Given
         mocked_repo.return_value = []
-        mocked_filters.side_effect = lambda f: f
         requested_id = 'unknown_id'
         repo = GeographyRepository()
 
@@ -127,11 +115,9 @@ class TestGeographyRepo(object):
             repo.get_by_id(requested_id)
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_slug(self, mocked_filters, mocked_repo):
+    def test_get_by_slug(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1]
-        mocked_filters.side_effect = lambda f: f
         requested_slug = db_geography1['slug']
         repo = GeographyRepository()
 
@@ -143,11 +129,9 @@ class TestGeographyRepo(object):
         assert geography == test_geography1
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_id_list(self, mocked_filters, mocked_repo):
+    def test_get_by_id_list(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1, db_geography2]
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
 
         # When
@@ -159,11 +143,9 @@ class TestGeographyRepo(object):
         assert geographies == test_geographies
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_slug_list(self, mocked_filters, mocked_repo):
+    def test_get_by_slug_list(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1, db_geography2]
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
 
         # When
@@ -175,11 +157,9 @@ class TestGeographyRepo(object):
         assert geographies == test_geographies
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_by_slug_and_id_list(self, mocked_filters, mocked_repo):
+    def test_get_by_slug_and_id_list(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1, db_geography2]
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
 
         # When
@@ -191,11 +171,9 @@ class TestGeographyRepo(object):
         assert geographies == test_geographies
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_get_all_with_join_filters(self, mocked_filters, mocked_repo):
+    def test_get_all_with_join_filters(self, mocked_repo):
         # Given
         mocked_repo.return_value = [db_geography1, db_geography2]
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
 
         # When
@@ -207,11 +185,9 @@ class TestGeographyRepo(object):
         assert geographies == test_geographies
 
     @patch.object(RepoClient, 'get_geographies')
-    @patch.object(RepoClient, 'get_entities_filters')
-    def test_missing_fields_are_mapped_as_None(self, mocked_filters, mocked_repo):
+    def test_missing_fields_are_mapped_as_None(self, mocked_repo):
         # Given
         mocked_repo.return_value = [{'id': 'geography1'}]
-        mocked_filters.side_effect = lambda f: f
         repo = GeographyRepository()
 
         expected_geographies = CatalogList([Geography({
diff --git a/tests/unit/data/observatory/catalog/test_catalog.py b/tests/unit/data/observatory/catalog/test_catalog.py
index 8d0b7803..ffdd590e 100644
--- a/tests/unit/data/observatory/catalog/test_catalog.py
+++ b/tests/unit/data/observatory/catalog/test_catalog.py
@@ -235,11 +235,11 @@ class TestCatalog(object):
         subscriptions = catalog.subscriptions(credentials)
 
         # Then
-        mocked_datasets.assert_called_once_with({}, credentials)
-        mocked_geographies.assert_called_once_with({}, credentials)
         assert isinstance(subscriptions, Subscriptions)
         assert subscriptions.datasets == expected_datasets
         assert subscriptions.geographies == expected_geographies
+        mocked_datasets.assert_called_once_with({}, credentials)
+        mocked_geographies.assert_called_once_with({}, credentials)
 
     @patch.object(Dataset, 'get_all')
     @patch.object(Geography, 'get_all')
@@ -258,11 +258,11 @@ class TestCatalog(object):
         subscriptions = catalog.subscriptions()
 
         # Then
-        mocked_datasets.assert_called_once_with({}, expected_credentials)
-        mocked_geographies.assert_called_once_with({}, expected_credentials)
         assert isinstance(subscriptions, Subscriptions)
         assert subscriptions.datasets == expected_datasets
         assert subscriptions.geographies == expected_geographies
+        mocked_datasets.assert_called_once_with({}, expected_credentials)
+        mocked_geographies.assert_called_once_with({}, expected_credentials)
 
     @patch.object(Dataset, 'get_all')
     @patch.object(Geography, 'get_all')