feat: invalid hostname and password error messages (MySQL) #14089

AAfghahi · 2021-04-12T21:14:17Z

SUMMARY

Capture the most common error messages in MySQL, and return them to the user. This is used when first making a DB.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

Created unit tests

ADDITIONAL INFORMATION

Has associated issue:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- 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

docs/src/pages/docs/Miscellaneous/issue_codes.mdx

superset-frontend/package-lock.json

betodealmeida · 2021-04-12T22:06:18Z

superset/db_engine_specs/mysql.py

@@ -93,6 +104,21 @@ class MySQLEngineSpec(BaseEngineSpec):

    type_code_map: Dict[int, str] = {}  # loaded from get_datatype only if needed

+    custom_errors = {
+        INVALID_ACCESS_REGEX: (
+            __('Either the username "%(username)s" or password is incorrect'),


"... or the password is incorrect." would be better?

Also, all messages should end in a period.

superset/errors.py

betodealmeida · 2021-04-12T22:07:34Z

tests/db_engine_specs/mysql_tests.py

+            )
+        ]
+
+        msg = "mysql: Can't connect to MySQL server on 'badconnection.com'"


Can you try also with an IP address instead of a name? To see if the message is similar and still gets captured?

psql: error: could not connect to server: Operation timed out Is the server running on host "93.184.216.34" and accepting TCP/IP connections on port 12345? """ ) result = PostgresEngineSpec.extract_errors(Exception(msg)) assert result == [ SupersetError( error_type=SupersetErrorType.TEST_CONNECTION_HOST_DOWN_ERROR, message=( "The host 93.184.216.34 might be down, " "and can't be reached on port 12345" ), level=ErrorLevel.ERROR, extra={"engine_name": "PostgreSQL"}, ) ]

Like this one? From your PR

For Postgres I tested with both a hostname and an IP, and made sure the regex captured both.

betodealmeida

A few comments.

betodealmeida · 2021-04-12T22:33:30Z

docs/src/pages/docs/Miscellaneous/issue_codes.mdx

+Either the username or the password used are incorrect.
 ```

-The user provided a password that is incorrect. Please check that the
-password is typed correctly.
+Either the username provided does not exist or the password was written incorrectly. Please
+check that the username and were typed correctly.


I merged issue 1013 already, so your PR is trying to replace it. You need to use 1014 now.

betodealmeida · 2021-04-12T22:34:51Z

superset/db_engine_specs/mysql.py

+        INVALID_ACCESS_REGEX: (
+            __('Either the username "%(username)s" or the password is incorrect.'),
+            SupersetErrorType.TEST_CONNECTION_ACCESS_DENIED_ERROR,


Let's rename the regex to ACCESS_DENIED_REGEX, to be consistent with the other errors.

betodealmeida · 2021-04-12T22:35:03Z

superset/db_engine_specs/mysql.py

+            SupersetErrorType.TEST_CONNECTION_ACCESS_DENIED_ERROR,
+        ),
+        INVALID_HOSTNAME_REGEX: (
+            __('Unknown MySQL server host "%(hostname)s"'),


Period here as well.

betodealmeida · 2021-04-12T22:35:25Z

superset/db_engine_specs/mysql.py

+            __('Unknown MySQL server host "%(hostname)s"'),
+            SupersetErrorType.TEST_CONNECTION_INVALID_HOSTNAME_ERROR,
+        ),
+        CONNECTION_HOST_DOWN_REGEX: (


Same here, let's call this just HOST_DOWN_REGEX.

Ah, I just saw that this already existed, sorry.

But I think we should call this TEST_CONNECTION_HOST_DOWN_REGEX, to match the error type.

Let's use that pattern as we add regexes.

this makes sense

betodealmeida · 2021-04-12T22:36:31Z

superset/errors.py

@@ -164,14 +165,13 @@ class SupersetErrorType(str, Enum):
            ),
        },
    ],
-    SupersetErrorType.TEST_CONNECTION_INVALID_PASSWORD_ERROR: [
+    SupersetErrorType.TEST_CONNECTION_ACCESS_DENIED_ERROR: [


Same thing, your PR is trying to override an existing error. You need to change this to 1014 and add it below.

This still needs to be fixed.

betodealmeida · 2021-04-12T22:37:22Z

tests/db_engine_specs/mysql_tests.py

+            )
+        ]
+
+        msg = "mysql: Can't connect to MySQL server on 'badconnection.com'"


For Postgres I tested with both a hostname and an IP, and made sure the regex captured both.

codecov · 2021-04-12T23:58:00Z

Codecov Report

Merging #14089 (125e6a5) into master (7980b76) will increase coverage by 0.33%.
The diff coverage is 86.73%.

❗ Current head 125e6a5 differs from pull request most recent head b769fa4. Consider uploading reports for the commit b769fa4 to get more accurate results

@@            Coverage Diff             @@
##           master   #14089      +/-   ##
==========================================
+ Coverage   79.48%   79.81%   +0.33%     
==========================================
  Files         939      942       +3     
  Lines       47587    47716     +129     
  Branches     5949     6009      +60     
==========================================
+ Hits        37823    38083     +260     
+ Misses       9642     9512     -130     
+ Partials      122      121       -1

Flag	Coverage Δ
cypress	`56.35% <89.07%> (+0.30%)`	⬆️
hive	`80.47% <90.19%> (-0.03%)`	⬇️
mysql	`80.74% <90.19%> (-0.03%)`	⬇️
postgres	`80.77% <90.19%> (-0.01%)`	⬇️
presto	`80.50% <90.19%> (?)`
python	`81.34% <90.19%> (+0.13%)`	⬆️
sqlite	`80.37% <90.19%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...rset-frontend/src/components/ErrorMessage/types.ts	`100.00% <ø> (ø)`
...et-frontend/src/dashboard/actions/nativeFilters.ts	`60.00% <ø> (ø)`
...rontend/src/dashboard/components/DashboardGrid.jsx	`89.47% <ø> (ø)`
...src/dashboard/components/PropertiesModal/index.jsx	`85.61% <ø> (ø)`
.../src/dashboard/components/gridComponents/Chart.jsx	`86.59% <ø> (ø)`
...nativeFilters/FilterBar/FilterSets/EditSection.tsx	`80.00% <ø> (+46.00%)`	⬆️
...tiveFilters/FilterBar/FilterSets/FilterSetUnit.tsx	`95.65% <ø> (+52.17%)`	⬆️
...tiveFilters/FilterBar/FilterSets/FiltersHeader.tsx	`91.42% <ø> (+59.99%)`	⬆️
...ents/nativeFilters/FilterBar/FilterSets/Footer.tsx	`95.23% <ø> (+47.61%)`	⬆️
...nents/nativeFilters/FilterBar/FilterSets/index.tsx	`46.46% <ø> (ø)`
... and 68 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 786dadc...b769fa4. Read the comment docs.

betodealmeida · 2021-04-13T17:07:42Z

docs/src/pages/docs/Miscellaneous/issue_codes.mdx

+## Issue 1014

 ```
-The password provided when connecting to a database is not valid.
+Either the username or the password used are incorrect.
 ```

-The user provided a password that is incorrect. Please check that the
-password is typed correctly.
+Either the username provided does not exist or the password was written incorrectly. Please
+check that the username and password were typed correctly.


This is still overwriting an existing issue.

betodealmeida · 2021-04-13T17:12:56Z

superset/errors.py

@@ -164,14 +165,13 @@ class SupersetErrorType(str, Enum):
            ),
        },
    ],
-    SupersetErrorType.TEST_CONNECTION_INVALID_PASSWORD_ERROR: [
+    SupersetErrorType.TEST_CONNECTION_ACCESS_DENIED_ERROR: [


This still needs to be fixed.

betodealmeida · 2021-04-13T17:15:16Z

superset/db_engine_specs/mysql.py

+            SupersetErrorType.TEST_CONNECTION_INVALID_HOSTNAME_ERROR,
+        ),
+        TEST_CONNECTION_HOST_DOWN_REGEX: (
+            __("The host %(hostname)s might be down and can't be reached."),


Let's be consistent and always quote the hostname (I need to do the same on my PR for MSSQL).

betodealmeida

Looks great!

) * custom errors for mySQL * initial custom errors and tests for MySQL * revisions

custom errors for mySQL

55de25c

AAfghahi force-pushed the ch6434mysql branch from 40c048b to 6763a02 Compare April 12, 2021 21:17

pull-request-size bot added the size/L label Apr 12, 2021

betodealmeida requested changes Apr 12, 2021

View reviewed changes

initial custom errors and tests for MySQL

7fcb78b

AAfghahi force-pushed the ch6434mysql branch from 6763a02 to 7fcb78b Compare April 12, 2021 22:13

betodealmeida requested changes Apr 12, 2021

View reviewed changes

junlincc added the data:connect:mysql Related to MySQL label Apr 13, 2021

AAfghahi force-pushed the ch6434mysql branch from b1cebda to 3d70a8b Compare April 13, 2021 17:00

betodealmeida requested changes Apr 13, 2021

View reviewed changes

AAfghahi force-pushed the ch6434mysql branch 3 times, most recently from 5f42af2 to 9f922cb Compare April 13, 2021 17:37

revisions

b769fa4

AAfghahi force-pushed the ch6434mysql branch from 9f922cb to b769fa4 Compare April 13, 2021 17:39

betodealmeida approved these changes Apr 13, 2021

View reviewed changes

betodealmeida merged commit b77477a into apache:master Apr 13, 2021

QAlexBall pushed a commit to QAlexBall/superset that referenced this pull request Dec 29, 2021

feat: invalid hostname and password error messages (MySQL) (apache#14089

8697b80

) * custom errors for mySQL * initial custom errors and tests for MySQL * revisions

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: invalid hostname and password error messages (MySQL) #14089

feat: invalid hostname and password error messages (MySQL) #14089

AAfghahi commented Apr 12, 2021

betodealmeida Apr 12, 2021

betodealmeida Apr 12, 2021

AAfghahi Apr 12, 2021

betodealmeida Apr 12, 2021

betodealmeida left a comment

betodealmeida Apr 12, 2021

betodealmeida Apr 12, 2021

betodealmeida Apr 12, 2021

betodealmeida Apr 12, 2021

betodealmeida Apr 12, 2021

betodealmeida Apr 12, 2021

AAfghahi Apr 13, 2021

betodealmeida Apr 12, 2021

betodealmeida Apr 13, 2021

betodealmeida Apr 12, 2021

codecov bot commented Apr 12, 2021 •

edited

Loading

betodealmeida Apr 13, 2021

betodealmeida Apr 13, 2021

betodealmeida Apr 13, 2021

betodealmeida left a comment

feat: invalid hostname and password error messages (MySQL) #14089

feat: invalid hostname and password error messages (MySQL) #14089

Conversation

AAfghahi commented Apr 12, 2021

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 12, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

betodealmeida left a comment

Choose a reason for hiding this comment

codecov bot commented Apr 12, 2021 •

edited

Loading