Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: added BasicParametersMixin to Redshift #14752

Merged
merged 3 commits into from
May 24, 2021
Merged

chore: added BasicParametersMixin to Redshift #14752

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
40cfa50
redshift basic mixin and tests
AAfghahi May 21, 2021
48668fd
rebased
AAfghahi May 24, 2021
67e1e9e
forgot prefix
AAfghahi May 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions superset/db_engine_specs/redshift.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from flask_babel import gettext as __

from superset.db_engine_specs.base import BasicParametersMixin
from superset.db_engine_specs.postgres import PostgresBaseEngineSpec
from superset.errors import SupersetErrorType

Expand All @@ -45,42 +46,48 @@
)


class RedshiftEngineSpec(PostgresBaseEngineSpec):
class RedshiftEngineSpec(PostgresBaseEngineSpec, BasicParametersMixin):
engine = "redshift"
engine_name = "Amazon Redshift"
max_column_name_length = 127

sqlalchemy_uri_placeholder = (
"redshift+psycopg2://user:password@host:port/dbname[?key=value&key=value...]"
)

encryption_parameters = {"sslmode": "verify-ca"}

custom_errors: Dict[Pattern[str], Tuple[str, SupersetErrorType, Dict[str, Any]]] = {
CONNECTION_ACCESS_DENIED_REGEX: (
__('Either the username "%(username)s" or the password is incorrect.'),
SupersetErrorType.CONNECTION_ACCESS_DENIED_ERROR,
{},
{"invalid": ["username", "password"]},
),
CONNECTION_INVALID_HOSTNAME_REGEX: (
__('The hostname "%(hostname)s" cannot be resolved.'),
SupersetErrorType.CONNECTION_INVALID_HOSTNAME_ERROR,
{},
{"invalid": ["host"]},
),
CONNECTION_PORT_CLOSED_REGEX: (
__('Port %(port)s on hostname "%(hostname)s" refused the connection.'),
SupersetErrorType.CONNECTION_PORT_CLOSED_ERROR,
{},
{"invalid": ["host", "port"]},
),
CONNECTION_HOST_DOWN_REGEX: (
__(
'The host "%(hostname)s" might be down, and can\'t be '
"reached on port %(port)s."
),
SupersetErrorType.CONNECTION_HOST_DOWN_ERROR,
{},
{"invalid": ["host", "port"]},
),
CONNECTION_UNKNOWN_DATABASE_REGEX: (
__(
'We were unable to connect to your database named "%(database)s".'
" Please verify your database name and try again."
),
SupersetErrorType.CONNECTION_UNKNOWN_DATABASE_ERROR,
{},
{"invalid": ["database"]},
),
}

Expand Down
95 changes: 61 additions & 34 deletions tests/databases/api_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from superset.connectors.sqla.models import SqlaTable
from superset.db_engine_specs.mysql import MySQLEngineSpec
from superset.db_engine_specs.postgres import PostgresEngineSpec
from superset.db_engine_specs.redshift import RedshiftEngineSpec
from superset.db_engine_specs.bigquery import BigQueryEngineSpec
from superset.db_engine_specs.hana import HanaEngineSpec
from superset.errors import SupersetError
Expand Down Expand Up @@ -1371,10 +1372,14 @@ def test_function_names(self, mock_get_function_names):
@mock.patch("superset.databases.api.get_available_engine_specs")
@mock.patch("superset.databases.api.app")
def test_available(self, app, get_available_engine_specs):
app.config = {"PREFERRED_DATABASES": ["postgresql"]}
app.config = {
"PREFERRED_DATABASES": ["postgresql", "biqquery", "mysql", "redshift"]
}
get_available_engine_specs.return_value = [
PostgresEngineSpec,
BigQueryEngineSpec,
MySQLEngineSpec,
RedshiftEngineSpec,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have a big query test here as well, make sure you rebase

HanaEngineSpec,
]

Expand Down Expand Up @@ -1431,46 +1436,52 @@ def test_available(self, app, get_available_engine_specs):
"sqlalchemy_uri_placeholder": "postgresql+psycopg2://user:password@host:port/dbname[?key=value&key=value...]",
},
{
"engine": "bigquery",
"name": "Google BigQuery",
"engine": "mysql",
"name": "MySQL",
"parameters": {
"properties": {
"credentials_info": {
"description": "Contents of BigQuery JSON credentials.",
"database": {
"description": "Database name",
"type": "string",
"x-encrypted-extra": True,
}
},
"encryption": {
"description": "Use an encrypted connection to the database",
"type": "boolean",
},
"host": {
"description": "Hostname or IP address",
"type": "string",
},
"password": {
"description": "Password",
"nullable": True,
"type": "string",
},
"port": {
"description": "Database port",
"format": "int32",
"type": "integer",
},
"query": {
"additionalProperties": {},
"description": "Additional parameters",
"type": "object",
},
"username": {
"description": "Username",
"nullable": True,
"type": "string",
},
},
"required": ["database", "host", "port", "username"],
"type": "object",
},
"preferred": False,
"sqlalchemy_uri_placeholder": "bigquery://{project_id}",
"preferred": True,
"sqlalchemy_uri_placeholder": "mysql://user:password@host:port/dbname[?key=value&key=value...]",
},
{"engine": "hana", "name": "SAP HANA", "preferred": False},
]
}

@mock.patch("superset.databases.api.get_available_engine_specs")
@mock.patch("superset.databases.api.app")
def test_available_with_mysql(self, app, get_available_engine_specs):
app.config = {"PREFERRED_DATABASES": ["mysql"]}
get_available_engine_specs.return_value = [
MySQLEngineSpec,
HanaEngineSpec,
]

self.login(username="admin")
uri = "api/v1/database/available/"

rv = self.client.get(uri)
response = json.loads(rv.data.decode("utf-8"))
print(response)
assert rv.status_code == 200
assert response == {
"databases": [
{
"engine": "mysql",
"name": "MySQL",
"engine": "redshift",
"name": "Amazon Redshift",
"parameters": {
"properties": {
"database": {
Expand Down Expand Up @@ -1510,7 +1521,23 @@ def test_available_with_mysql(self, app, get_available_engine_specs):
"type": "object",
},
"preferred": True,
"sqlalchemy_uri_placeholder": "mysql://user:password@host:port/dbname[?key=value&key=value...]",
"sqlalchemy_uri_placeholder": "redshift+psycopg2://user:password@host:port/dbname[?key=value&key=value...]",
},
{
"engine": "bigquery",
"name": "Google BigQuery",
"parameters": {
"properties": {
"credentials_info": {
"description": "Contents of BigQuery JSON credentials.",
"type": "string",
"x-encrypted-extra": True,
}
},
"type": "object",
},
"preferred": False,
"sqlalchemy_uri_placeholder": "bigquery://{project_id}",
},
{"engine": "hana", "name": "SAP HANA", "preferred": False},
]
Expand Down
6 changes: 6 additions & 0 deletions tests/db_engine_specs/redshift_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def test_extract_errors(self):
message='Either the username "wronguser" or the password is incorrect.',
level=ErrorLevel.ERROR,
extra={
"invalid": ["username", "password"],
"engine_name": "Amazon Redshift",
"issue_codes": [
{
Expand Down Expand Up @@ -62,6 +63,7 @@ def test_extract_errors(self):
message='The hostname "badhost" cannot be resolved.',
level=ErrorLevel.ERROR,
extra={
"invalid": ["host"],
"engine_name": "Amazon Redshift",
"issue_codes": [
{
Expand Down Expand Up @@ -90,6 +92,7 @@ def test_extract_errors(self):
message='Port 12345 on hostname "localhost" refused the connection.',
level=ErrorLevel.ERROR,
extra={
"invalid": ["host", "port"],
"engine_name": "Amazon Redshift",
"issue_codes": [
{"code": 1008, "message": "Issue 1008 - The port is closed."}
Expand Down Expand Up @@ -123,6 +126,7 @@ def test_extract_errors(self):
"and can't be reached on the provided port.",
}
],
"invalid": ["host", "port"],
},
)
]
Expand Down Expand Up @@ -153,6 +157,7 @@ def test_extract_errors(self):
"and can't be reached on the provided port.",
}
],
"invalid": ["host", "port"],
},
)
]
Expand All @@ -174,6 +179,7 @@ def test_extract_errors(self):
"spelled incorrectly or does not exist.",
}
],
"invalid": ["database"],
},
)
]