You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/11/03 12:37:51 UTC

[GitHub] [superset] Antonio-RiveroMartnez opened a new pull request, #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Antonio-RiveroMartnez opened a new pull request, #22024:
URL: https://github.com/apache/superset/pull/22024

   ### SUMMARY
   Some BigQuery exceptions are not being logged with enough information, so we need to add some extra method that parses the original exception and logs the correct message. We also add some unit tests to the new method.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   After:
   ![test](https://user-images.githubusercontent.com/38889534/199721944-5717e9b9-698c-406b-90c6-0b4a4bd4c2a3.png)
   
   
   ### TESTING INSTRUCTIONS
   1. Connect Locally to a BigQuery DB
   2. Run an invalid SQL statement so you get a BigQuery exception
   3. Check you are logging the correct message out of the original exception
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] hughhhh commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015723744


##########
superset/utils/core.py:
##########
@@ -190,6 +190,10 @@ class DatasourceType(str, Enum):
     VIEW = "view"
 
 
+class EngineType(str, Enum):

Review Comment:
   πŸ‘πŸΎ 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez closed pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez closed pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions
URL: https://github.com/apache/superset/pull/22024


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] hughhhh commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015929654


##########
superset/db_engine_specs/base.py:
##########
@@ -430,6 +430,15 @@ def get_dbapi_exception_mapping(cls) -> Dict[Type[Exception], Type[Exception]]:
         """
         return {}
 
+    @classmethod
+    def parse_error_exception(cls, exception: Exception) -> Exception:
+        """
+        Each engine can implement and converge its own specific parser method
+
+        :return: An Exception with a parsed string off the original exception
+        """
+        return exception

Review Comment:
   make this NotImplemented()



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] hughhhh merged pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
hughhhh merged PR #22024:
URL: https://github.com/apache/superset/pull/22024


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] hughhhh commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015720548


##########
superset/db_engine_specs/base.py:
##########
@@ -443,6 +452,9 @@ def get_dbapi_mapped_exception(cls, exception: Exception) -> Exception:
         """
         new_exception = cls.get_dbapi_exception_mapping().get(type(exception))
         if not new_exception:
+            # We are interested in just BigQuery exceptions
+            if cls.engine == EngineType.BIGQUERY:

Review Comment:
   a better check is if the class has the function `parse_error_exception`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015722785


##########
superset/db_engine_specs/base.py:
##########
@@ -443,6 +452,9 @@ def get_dbapi_mapped_exception(cls, exception: Exception) -> Exception:
         """
         new_exception = cls.get_dbapi_exception_mapping().get(type(exception))
         if not new_exception:
+            # We are interested in just BigQuery exceptions
+            if cls.engine == EngineType.BIGQUERY:

Review Comment:
   They all have it because the base class have it, I had it that way initially but our hooks and linting rules don't like it because `cls` would be the base class and it checks with it instead of the BigQuery one for example. So, Was that or ignoring the rule if possible.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1013162433


##########
superset/db_engine_specs/base.py:
##########
@@ -443,6 +452,9 @@ def get_dbapi_mapped_exception(cls, exception: Exception) -> Exception:
         """
         new_exception = cls.get_dbapi_exception_mapping().get(type(exception))
         if not new_exception:
+            # We are interested in just BigQuery exceptions
+            if cls.engine == "bigquery":

Review Comment:
   Hmm, that's in the `superset-frontend` module, right? Do we have some similar enum in Python?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] hughhhh commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015928999


##########
tests/unit_tests/db_engine_specs/test_bigquery.py:
##########
@@ -244,3 +244,74 @@ def test_mask_encrypted_extra_when_empty() -> None:
     from superset.db_engine_specs.bigquery import BigQueryEngineSpec
 
     assert BigQueryEngineSpec.mask_encrypted_extra(None) is None
+
+
+def test_parse_error_message() -> None:
+    """
+    Test that we parse a received message and just extract the useful information.
+
+    Example errors:
+    400 Syntax error: Expected "(" or keyword UNNEST but got "@" at [4:80]
+
+    (job ID: 60c732c3-4b4e-4da6-9988-18ba20d389ee)
+
+                                                -----Query Job SQL Follows-----
+
+    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |
+    1:SELECT count(*) AS `count_1`
+    2:FROM (SELECT `bigquery-public-data.census_bureau_acs.blockgroup_2010_5yr`.`geo_id` AS `bigquery_public_data_census_bureau_acs_blockgroup_2010_5yr_geo_id`
+    3:FROM `bigquery-public-data.census_bureau_acs.blockgroup_2010_5yr`
+    4:WHERE `bigquery-public-data.census_bureau_acs.blockgroup_2010_5yr`.`geo_id` IN @`geo_id_1`) AS `anon_1`
+    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |
+
+
+    bigquery error: 400 Table \"case_detail_all_suites\" must be qualified with a dataset (e.g. dataset.table).

Review Comment:
   make all the test work with this example



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] codecov[bot] commented on pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22024:
URL: https://github.com/apache/superset/pull/22024#issuecomment-1302177189

   # [Codecov](https://codecov.io/gh/apache/superset/pull/22024?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#22024](https://codecov.io/gh/apache/superset/pull/22024?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6f7bda4) into [master](https://codecov.io/gh/apache/superset/commit/4cbd70db34b140a026ef1a86a8ef0ba3355a350e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4cbd70d) will **decrease** coverage by `0.92%`.
   > The diff coverage is `75.00%`.
   
   > :exclamation: Current head 6f7bda4 differs from pull request most recent head b6fbdd6. Consider uploading reports for the commit b6fbdd6 to get more accurate results
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #22024      +/-   ##
   ==========================================
   - Coverage   67.03%   66.11%   -0.93%     
   ==========================================
     Files        1813     1813              
     Lines       69424    69432       +8     
     Branches     7448     7448              
   ==========================================
   - Hits        46541    45902     -639     
   - Misses      20963    21610     +647     
     Partials     1920     1920              
   ```
   
   | Flag | Coverage Ξ” | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `?` | |
   | python | `79.61% <75.00%> (-1.93%)` | :arrow_down: |
   | sqlite | `76.90% <62.50%> (-0.01%)` | :arrow_down: |
   | unit | `51.18% <62.50%> (+<0.01%)` | :arrow_up: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/22024?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ” | |
   |---|---|---|
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.47% <60.00%> (-0.70%)` | :arrow_down: |
   | [superset/db\_engine\_specs/bigquery.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2JpZ3F1ZXJ5LnB5) | `82.32% <100.00%> (+0.27%)` | :arrow_up: |
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.19%)` | :arrow_down: |
   | [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `30.61% <0.00%> (-69.39%)` | :arrow_down: |
   | [superset/datasets/commands/update.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvdXBkYXRlLnB5) | `25.00% <0.00%> (-69.05%)` | :arrow_down: |
   | [superset/datasets/commands/bulk\_delete.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvYnVsa19kZWxldGUucHk=) | `33.33% <0.00%> (-53.34%)` | :arrow_down: |
   | [superset/datasets/columns/commands/delete.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29sdW1ucy9jb21tYW5kcy9kZWxldGUucHk=) | `44.11% <0.00%> (-52.95%)` | :arrow_down: |
   | [superset/datasets/metrics/commands/delete.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvbWV0cmljcy9jb21tYW5kcy9kZWxldGUucHk=) | `44.11% <0.00%> (-52.95%)` | :arrow_down: |
   | [superset/datasets/dao.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvZGFvLnB5) | `44.21% <0.00%> (-50.35%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | ... and [34 more](https://codecov.io/gh/apache/superset/pull/22024/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] hughhhh commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1013158699


##########
superset/db_engine_specs/bigquery.py:
##########
@@ -578,3 +578,7 @@ def _get_fields(cls, cols: List[Dict[str, Any]]) -> List[Any]:
         "author__name" and "author__email", respectively.
         """
         return [column(c["name"]).label(c["name"].replace(".", "__")) for c in cols]
+
+    @classmethod
+    def parse_error_exception(cls, exception_message: str) -> str:
+        return exception_message.rsplit("\n")[0]

Review Comment:
   Can we have better try/exception catching here just incase we can't find the new line? Then a write test to back this whenever `parse_error_exception` raises.



##########
superset/db_engine_specs/base.py:
##########
@@ -443,6 +452,9 @@ def get_dbapi_mapped_exception(cls, exception: Exception) -> Exception:
         """
         new_exception = cls.get_dbapi_exception_mapping().get(type(exception))
         if not new_exception:
+            # We are interested in just BigQuery exceptions
+            if cls.engine == "bigquery":

Review Comment:
   can we add `bigquery` to this engine + add it here as `Engines.BigQuery`
   
   https://github.com/apache/superset/blob/66c0801e494f16ba44d492f3648c1e1a57c8e35a/superset-frontend/src/views/CRUD/data/database/types.ts#L150



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015722785


##########
superset/db_engine_specs/base.py:
##########
@@ -443,6 +452,9 @@ def get_dbapi_mapped_exception(cls, exception: Exception) -> Exception:
         """
         new_exception = cls.get_dbapi_exception_mapping().get(type(exception))
         if not new_exception:
+            # We are interested in just BigQuery exceptions
+            if cls.engine == EngineType.BIGQUERY:

Review Comment:
   They all have it because the base class have it, I had it that way initially but our hooks and linting rules don't like it because `cls` would be the base class and it checks with it instead of the BigQuery one for example. So, Was that or ignoring the rule if possible.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] eschutho commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1018541443


##########
superset/db_engine_specs/bigquery.py:
##########
@@ -578,3 +578,17 @@ def _get_fields(cls, cols: List[Dict[str, Any]]) -> List[Any]:
         "author__name" and "author__email", respectively.
         """
         return [column(c["name"]).label(c["name"].replace(".", "__")) for c in cols]
+
+    @classmethod
+    def parse_error_exception(cls, exception: Exception) -> Exception:
+        try:
+            return Exception(
+                str(exception)  # pylint: disable=use-maxsplit-arg
+                .rsplit("\n")[0]
+                .rsplit(":")[1]
+                .strip()
+            )
+        except Exception:  # pylint: disable=broad-except

Review Comment:
   Oh, ok I tried it with no new line. That makes sense. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez closed pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez closed pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions
URL: https://github.com/apache/superset/pull/22024


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] hughhhh commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015720548


##########
superset/db_engine_specs/base.py:
##########
@@ -443,6 +452,9 @@ def get_dbapi_mapped_exception(cls, exception: Exception) -> Exception:
         """
         new_exception = cls.get_dbapi_exception_mapping().get(type(exception))
         if not new_exception:
+            # We are interested in just BigQuery exceptions
+            if cls.engine == EngineType.BIGQUERY:

Review Comment:
   let's just return `parse_error_exception` and we'll log this error upstream



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] eschutho commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1017258956


##########
superset/db_engine_specs/bigquery.py:
##########
@@ -578,3 +578,17 @@ def _get_fields(cls, cols: List[Dict[str, Any]]) -> List[Any]:
         "author__name" and "author__email", respectively.
         """
         return [column(c["name"]).label(c["name"].replace(".", "__")) for c in cols]
+
+    @classmethod
+    def parse_error_exception(cls, exception: Exception) -> Exception:
+        try:
+            return Exception(
+                str(exception)  # pylint: disable=use-maxsplit-arg
+                .rsplit("\n")[0]
+                .rsplit(":")[1]
+                .strip()
+            )
+        except Exception:  # pylint: disable=broad-except

Review Comment:
   I tried to get this to raise an exception and wasn't able to. I believe as long as the return value is a string, you should be able to perform all of the operations and don't need to put it in a try catch. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez closed pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez closed pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions
URL: https://github.com/apache/superset/pull/22024


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1015949367


##########
tests/unit_tests/db_engine_specs/test_bigquery.py:
##########
@@ -244,3 +244,74 @@ def test_mask_encrypted_extra_when_empty() -> None:
     from superset.db_engine_specs.bigquery import BigQueryEngineSpec
 
     assert BigQueryEngineSpec.mask_encrypted_extra(None) is None
+
+
+def test_parse_error_message() -> None:
+    """
+    Test that we parse a received message and just extract the useful information.
+
+    Example errors:
+    400 Syntax error: Expected "(" or keyword UNNEST but got "@" at [4:80]
+
+    (job ID: 60c732c3-4b4e-4da6-9988-18ba20d389ee)
+
+                                                -----Query Job SQL Follows-----
+
+    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |
+    1:SELECT count(*) AS `count_1`
+    2:FROM (SELECT `bigquery-public-data.census_bureau_acs.blockgroup_2010_5yr`.`geo_id` AS `bigquery_public_data_census_bureau_acs_blockgroup_2010_5yr_geo_id`
+    3:FROM `bigquery-public-data.census_bureau_acs.blockgroup_2010_5yr`
+    4:WHERE `bigquery-public-data.census_bureau_acs.blockgroup_2010_5yr`.`geo_id` IN @`geo_id_1`) AS `anon_1`
+    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |    .    |
+
+
+    bigquery error: 400 Table \"case_detail_all_suites\" must be qualified with a dataset (e.g. dataset.table).

Review Comment:
   Updated πŸ‘ 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] eschutho commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
eschutho commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1017258333


##########
superset/db_engine_specs/bigquery.py:
##########
@@ -578,3 +578,17 @@ def _get_fields(cls, cols: List[Dict[str, Any]]) -> List[Any]:
         "author__name" and "author__email", respectively.
         """
         return [column(c["name"]).label(c["name"].replace(".", "__")) for c in cols]
+
+    @classmethod
+    def parse_error_exception(cls, exception: Exception) -> Exception:
+        try:
+            return Exception(
+                str(exception)  # pylint: disable=use-maxsplit-arg
+                .rsplit("\n")[0]

Review Comment:
   you can also use `.splitlines()[0]` here



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

[GitHub] [superset] Antonio-RiveroMartnez commented on a diff in pull request #22024: chore(bigquery): Add extra logging for BigQuery exceptions so we can have better insight on exceptions

Posted by GitBox <gi...@apache.org>.
Antonio-RiveroMartnez commented on code in PR #22024:
URL: https://github.com/apache/superset/pull/22024#discussion_r1017299068


##########
superset/db_engine_specs/bigquery.py:
##########
@@ -578,3 +578,17 @@ def _get_fields(cls, cols: List[Dict[str, Any]]) -> List[Any]:
         "author__name" and "author__email", respectively.
         """
         return [column(c["name"]).label(c["name"].replace(".", "__")) for c in cols]
+
+    @classmethod
+    def parse_error_exception(cls, exception: Exception) -> Exception:
+        try:
+            return Exception(
+                str(exception)  # pylint: disable=use-maxsplit-arg
+                .rsplit("\n")[0]
+                .rsplit(":")[1]
+                .strip()
+            )
+        except Exception:  # pylint: disable=broad-except

Review Comment:
   Hmm if for some unknown reason the Exception has no `:` for example, accessing the [1] would cause a: `IndexError: list index out of range`.  Just adding the try/catch there in case anything like that happens with the message we get.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org