From 16227a8a345dfe0408f782fa41b38eb17f8c54eb Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Tue, 16 Dec 2025 16:25:02 -0300 Subject: [PATCH 1/3] [python-package]: improve errors Close #1743 --- python-package/basedosdados/backend.py | 8 +++---- .../basedosdados/download/download.py | 12 ++++++++-- python-package/basedosdados/upload/table.py | 24 +------------------ python-package/tests/test_download.py | 15 +++--------- 4 files changed, 18 insertions(+), 41 deletions(-) diff --git a/python-package/basedosdados/backend.py b/python-package/basedosdados/backend.py index ef698a028..e89c3fbd0 100644 --- a/python-package/basedosdados/backend.py +++ b/python-package/basedosdados/backend.py @@ -507,11 +507,11 @@ def _execute_query( try: response = client_.execute(gql(query), variable_values=variables) except Exception as e: - logger.error( - f"The API URL in the config.toml file may be incorrect " - f"or the API might be temporarily unavailable!\n" - f"Error executing query: {e}." + msg = ( + "The API URL in the config.toml file may be incorrect " + "or the API might be temporarily unavailable!\n" ) + raise BaseDosDadosException(msg) from e return self._simplify_response(response or {}, page, page_size) def _simplify_response( diff --git a/python-package/basedosdados/download/download.py b/python-package/basedosdados/download/download.py index b96845160..e5153554b 100644 --- a/python-package/basedosdados/download/download.py +++ b/python-package/basedosdados/download/download.py @@ -40,6 +40,14 @@ def _set_config_variables( from_file: bool, ) -> tuple[str, bool]: """Set billing_project_id and from_file variables.""" + + if ( + billing_project_id is None + and config.billing_project_id is None + and not from_file + ): + raise BaseDosDadosNoBillingProjectIDException + # standard billing_project_id configuration billing_project_id = billing_project_id or config.billing_project_id # standard from_file configuration @@ -104,7 +112,7 @@ def read_sql( if re.match("Reason: 400 POST .* [Pp]roject[ ]*I[Dd]", str(e)): raise BaseDosDadosInvalidProjectIDException from e - raise + raise e except PyDataCredentialsError as e: raise BaseDosDadosAuthorizationException from e @@ -116,7 +124,7 @@ def read_sql( ) if no_billing_id: raise BaseDosDadosNoBillingProjectIDException from e - raise + raise e def read_table( diff --git a/python-package/basedosdados/upload/table.py b/python-package/basedosdados/upload/table.py index f1aeff70d..963530491 100644 --- a/python-package/basedosdados/upload/table.py +++ b/python-package/basedosdados/upload/table.py @@ -298,7 +298,7 @@ def _get_columns_from_bq( """ if not self.table_exists(mode=mode): msg = f"Table {self.dataset_id}.{self.table_id} does not exist in {mode}, please create first!" - raise logger.error(msg) + raise BaseDosDadosException(msg) else: schema = self._get_table_obj(mode=mode).schema @@ -441,19 +441,11 @@ def _get_biglake_connection( connection.create() logger.success("BigLake connection created!") except google.api_core.exceptions.Forbidden as exc: - logger.error( - "You don't have permission to create a BigLake connection. " - "Please contact an admin to create one for you." - ) raise BaseDosDadosException( "You don't have permission to create a BigLake connection. " "Please contact an admin to create one for you." ) from exc except Exception as exc: - logger.error( - "Something went wrong while creating the BigLake connection. " - "Please contact an admin to create one for you." - ) raise BaseDosDadosException( "Something went wrong while creating the BigLake connection. " "Please contact an admin to create one for you." @@ -466,13 +458,6 @@ def _get_biglake_connection( connection.set_biglake_permissions() logger.success("Permissions set successfully!") except google.api_core.exceptions.Forbidden as exc: - logger.error( - "Could not set permissions for BigLake service account. " - "Please make sure you have permissions to grant roles/storage.objectViewer" - f" to the BigLake service account. ({connection.service_account})." - " If you don't, please ask an admin to do it for you or set " - "set_biglake_connection_permissions=False." - ) raise BaseDosDadosException( "Could not set permissions for BigLake service account. " "Please make sure you have permissions to grant roles/storage.objectViewer" @@ -481,13 +466,6 @@ def _get_biglake_connection( "set_biglake_connection_permissions=False." ) from exc except Exception as exc: - logger.error( - "Something went wrong while setting permissions for BigLake service account. " - "Please make sure you have permissions to grant roles/storage.objectViewer" - f" to the BigLake service account. ({connection.service_account})." - " If you don't, please ask an admin to do it for you or set " - "set_biglake_connection_permissions=False." - ) raise BaseDosDadosException( "Something went wrong while setting permissions for BigLake service account. " "Please make sure you have permissions to grant roles/storage.objectViewer" diff --git a/python-package/tests/test_download.py b/python-package/tests/test_download.py index 0a59fcd76..b1741249d 100644 --- a/python-package/tests/test_download.py +++ b/python-package/tests/test_download.py @@ -11,6 +11,7 @@ from basedosdados import download, read_sql, read_table from basedosdados.exceptions import ( + BaseDosDadosAccessDeniedException, BaseDosDadosException, ) @@ -54,7 +55,6 @@ def test_download_by_table(): assert SAVEFILE.exists() -@pytest.mark.skip(reason="Takes long time to run. Better run isolated.") def test_download_large_file(): """ Test for the `download` function for a large file when the query is @@ -63,7 +63,7 @@ def test_download_large_file(): download( SAVEFILE, - query="select * from basedosdados.br_me_rais.microdados_vinculos limit 10000000", + query="select * from basedosdados.br_me_rais.microdados_vinculos limit 10", billing_project_id=TEST_PROJECT_ID, from_file=True, ) @@ -113,28 +113,19 @@ def test_read_sql_invalid_billing_project_id(): ) -@pytest.mark.skip( - reason="TODO: Refactor the exceptions that are thrown in read_sql" -) def test_read_sql_inexistent_project(): """ Test if the `read_sql` function raises an error when the billing project id is not valid. """ - # this is the exception throw BaseDosDadosAccessDeniedException - with pytest.raises(GenericGBQException) as excinfo: + with pytest.raises(BaseDosDadosAccessDeniedException): read_sql( query="select * from `asedosdados.br_ibge_pib.municipio` limit 10", billing_project_id=TEST_PROJECT_ID, from_file=True, ) - # print('Exec Info Type Name: ') - # print(excinfo.typename) - - assert "Reason: 404 Not found: Project" in str(excinfo.value) - def test_read_sql_inexistent_dataset(): """ From 26d649c866e41dab7ed51bee093dc14f23bba334 Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Tue, 16 Dec 2025 17:52:45 -0300 Subject: [PATCH 2/3] remove call to `_simplify_response` --- python-package/basedosdados/backend.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/python-package/basedosdados/backend.py b/python-package/basedosdados/backend.py index e89c3fbd0..4fd8ec0da 100644 --- a/python-package/basedosdados/backend.py +++ b/python-package/basedosdados/backend.py @@ -302,10 +302,9 @@ def get_dataset_config(self, dataset_id: str) -> Dict[str, Any]: dataset_id = self._get_dataset_id_from_name(dataset_id) if dataset_id: variables = {"dataset_id": dataset_id} - response = self._execute_query(query=query, variables=variables) - return self._simplify_response(response).get("allDataset")[ - "items" - ][0] + return self._execute_query(query=query, variables=variables)[ + "allDataset" + ]["items"][0] else: return {} @@ -364,10 +363,9 @@ def get_table_config( if table_id: variables = {"table_id": table_id} - response = self._execute_query(query=query, variables=variables) - return self._simplify_response(response).get("allTable")["items"][ - 0 - ] + return self._execute_query(query=query, variables=variables)[ + "allTable" + ]["items"][0] else: return {} @@ -390,7 +388,7 @@ def _get_dataset_id_from_name(self, gcp_dataset_id): variables = {"gcp_dataset_id": gcp_dataset_id} response = self._execute_query(query=query, variables=variables) - r = {} if response is None else self._simplify_response(response) + r = {} if response is None else response if r.get("allCloudtable") != []: return ( r.get("allCloudtable")["items"][0] @@ -424,7 +422,7 @@ def _get_table_id_from_name(self, gcp_dataset_id, gcp_table_id): } response = self._execute_query(query=query, variables=variables) - r = {} if response is None else self._simplify_response(response) + r = {} if response is None else response if r.get("allCloudtable", []) != []: return ( r.get("allCloudtable")["items"][0].get("table").get("_id") From 4af301f194c53a000791450ab0b72fe3e6be78be Mon Sep 17 00:00:00 2001 From: Pedro Castro Date: Wed, 17 Dec 2025 10:25:09 -0300 Subject: [PATCH 3/3] fix parameters order --- python-package/basedosdados/backend.py | 43 +++++++++++++------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/python-package/basedosdados/backend.py b/python-package/basedosdados/backend.py index 4fd8ec0da..5e288e794 100644 --- a/python-package/basedosdados/backend.py +++ b/python-package/basedosdados/backend.py @@ -110,9 +110,9 @@ def get_datasets( if extra: query = query.replace("$offset)", f"$offset, {extra})") - return self._execute_query(query, variables, page, page_size).get( - "allDataset" - ) + return self._execute_query( + query=query, variables=variables, page=page, page_size=page_size + ).get("allDataset") def get_tables( self, @@ -169,9 +169,9 @@ def get_tables( if extra: query = query.replace("$offset)", f"$offset, {extra})") - return self._execute_query(query, variables, page, page_size).get( - "allTable" - ) + return self._execute_query( + query=query, variables=variables, page=page, page_size=page_size + ).get("allTable") def get_columns( self, @@ -227,9 +227,9 @@ def get_columns( if extra: query = query.replace("$offset)", f"$offset, {extra})") - return self._execute_query(query, variables, page, page_size).get( - "allColumn" - ) + return self._execute_query( + query=query, variables=variables, page=page, page_size=page_size + ).get("allColumn") def search( self, q: Optional[str] = None, page: int = 1, page_size: int = 10 @@ -299,9 +299,9 @@ def get_dataset_config(self, dataset_id: str) -> Dict[str, Any]: } } """ - dataset_id = self._get_dataset_id_from_name(dataset_id) - if dataset_id: - variables = {"dataset_id": dataset_id} + dataset_id_from_name = self._get_dataset_id_from_name(dataset_id) + if dataset_id_from_name is not None: + variables = {"dataset_id": dataset_id_from_name} return self._execute_query(query=query, variables=variables)[ "allDataset" ]["items"][0] @@ -357,19 +357,19 @@ def get_table_config( } } """ - table_id = self._get_table_id_from_name( + table_id_from_name = self._get_table_id_from_name( gcp_dataset_id=dataset_id, gcp_table_id=table_id ) - if table_id: - variables = {"table_id": table_id} + if table_id_from_name is not None: + variables = {"table_id": table_id_from_name} return self._execute_query(query=query, variables=variables)[ "allTable" ]["items"][0] else: return {} - def _get_dataset_id_from_name(self, gcp_dataset_id): + def _get_dataset_id_from_name(self, gcp_dataset_id: str) -> Optional[str]: query = """ query ($gcp_dataset_id: String!){ allCloudtable(gcpDatasetId: $gcp_dataset_id) { @@ -400,7 +400,9 @@ def _get_dataset_id_from_name(self, gcp_dataset_id): logger.info(msg) return None - def _get_table_id_from_name(self, gcp_dataset_id, gcp_table_id): + def _get_table_id_from_name( + self, gcp_dataset_id: str, gcp_table_id: str + ) -> Optional[str]: query = """ query ($gcp_dataset_id: String!, $gcp_table_id: String!){ allCloudtable(gcpDatasetId: $gcp_dataset_id, gcpTableId: $gcp_table_id) { @@ -424,9 +426,8 @@ def _get_table_id_from_name(self, gcp_dataset_id, gcp_table_id): response = self._execute_query(query=query, variables=variables) r = {} if response is None else response if r.get("allCloudtable", []) != []: - return ( - r.get("allCloudtable")["items"][0].get("table").get("_id") - ) + return r["allCloudtable"]["items"][0]["table"]["_id"] + msg = f"No table {gcp_table_id} found in {gcp_dataset_id}. Please create in {self.graphql_url}" logger.info(msg) return None @@ -465,7 +466,7 @@ def _get_client( def _execute_query( self, query: str, - variables: Optional[Dict[str, str]] = None, + variables: Optional[Dict[str, Any]] = None, client: Optional["Client"] = None, # type: ignore headers: Optional[Dict[str, str]] = None, page: int = 1,