betodealmeida commented on a change in pull request #14147:
URL: https://github.com/apache/superset/pull/14147#discussion_r616133624
##########
File path: superset/db_engine_specs/bigquery.py
##########
@@ -20,16 +20,23 @@
from typing import Any, Dict, List, Optional, Tuple, TYPE_CHECKING
import pandas as pd
+from flask_babel import gettext as __
from sqlalchemy import literal_column
from sqlalchemy.sql.expression import ColumnClause
from superset.db_engine_specs.base import BaseEngineSpec
+from superset.errors import SupersetErrorType
from superset.utils import core as utils
if TYPE_CHECKING:
from superset.models.core import Database # pragma: no cover
+CONNECTION_DATABASE_PERMISSIONS_REGEX = re.compile(
+ "(?:^|(?<= ))(403|POST|permission)(?:(?= )|$)"
Review comment:
I think this is too broad, it might catch other errors that are
unrelated.
It would be better to catch something like:
```python
"Access Denied: Project User does not have bigquery.jobs.create permission
in project (?P<project>.+?)"
```
##########
File path: superset/errors.py
##########
@@ -47,6 +47,7 @@ class SupersetErrorType(str, Enum):
CONNECTION_HOST_DOWN_ERROR = "CONNECTION_HOST_DOWN_ERROR"
CONNECTION_ACCESS_DENIED_ERROR = "CONNECTION_ACCESS_DENIED_ERROR"
CONNECTION_UNKNOWN_DATABASE_ERROR = "CONNECTION_UNKNOWN_DATABASE_ERROR"
+ CONNECTION_DATABASE_PERMISSIONS_ERROR =
"TEST_CONNECTION_DATABASE_PERMISSIONS_ERROR"
Review comment:
You also need to add this to:
- `superset-frontend/src/components/ErrorMessage/types.ts`
- `superset-frontend/src/setup/setupErrorMessages.ts`
- `docs/src/pages/docs/Miscellaneous/issue_codes.mdx`
##########
File path: docs/src/pages/docs/Miscellaneous/issue_codes.mdx
##########
@@ -167,11 +168,18 @@ Either the database is spelled incorrectly or does not
exist.
Either the database was written incorrectly or it does not exist. Check that
it was typed correctly.
-
## Issue 1016
```
The schema was deleted or renamed in the database.
```
The schema was either removed or renamed. Check that the schema is typed
correctly and exists.
+
+## Issue 1017
+
+```
+We were unable to connect to your database. Please confirm that your service
account has the Viewer and Job User roles on the project.
+```
+
+The user doesn't have the proper permissions to connect to the database
Review comment:
The convention we've been using is:
```
```
Short message describing the problem.
```
Descriptive message with actionable items.
```
So in this case it would be better to reverse the 2 messages you have here.
##########
File path: superset/db_engine_specs/bigquery.py
##########
@@ -86,6 +93,17 @@ class BigQueryEngineSpec(BaseEngineSpec):
"P1Y": "{func}({col}, YEAR)",
}
+ custom_errors = {
+ CONNECTION_DATABASE_PERMISSIONS_REGEX: (
+ __(
+ "We were unable to connect to your database. Please "
+ "confirm that your service account has the Viewer "
+ "and Job User roles on the project."
Review comment:
This should also be simple, just describing the error. The actionable
item should be in the issue code.
##########
File path: superset/errors.py
##########
@@ -213,6 +214,12 @@ class SupersetErrorType(str, Enum):
),
}
],
+ SupersetErrorType.CONNECTION_DATABASE_PERMISSIONS_ERROR: [
+ {
+ "code": 1017,
+ "message": _("Issue 1017 - User doesn't have the proper
permissions"),
Review comment:
```suggestion
"message": _("Issue 1017 - User doesn't have the proper
permissions."),
```
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
