Skip to content

Commit

Permalink
Merge pull request #140 from preset-io/masked_password_update
Browse files Browse the repository at this point in the history 
fix (native): no password prompt on db update
  • Loading branch information
@betodealmeida
betodealmeida authored Nov 1, 2022
2 parents 3ceedca + e2b2841 commit d99c9ab
Show file tree
Hide file tree
Showing 2 changed files with 127 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/preset_cli/cli/superset/sync/native/command.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import getpass
import importlib.util
import json
import logging
import os
from datetime import datetime, timezone
Expand Down Expand Up @@ -149,6 +150,9 @@ def native( # pylint: disable=too-many-locals, too-many-arguments

base_url = URL(external_url_prefix) if external_url_prefix else None

# collecting existing database UUIDs so we know if we're creating or updating
existing_databases = {str(uuid) for uuid in client.get_uuids("database").values()}

# env for Jinja2 templating
env = dict(pair.split("=", 1) for pair in option if "=" in pair) # type: ignore
env["instance"] = url
Expand Down Expand Up @@ -179,9 +183,16 @@ def native( # pylint: disable=too-many-locals, too-many-arguments
config["external_url"] = str(
base_url / str(relative_path),
)
if relative_path.parts[0] == "databases":
if (
relative_path.parts[0] == "databases"
and config["uuid"] not in existing_databases
):
prompt_for_passwords(relative_path, config)
verify_db_connectivity(config)
if relative_path.parts[0] == "datasets" and isinstance(
config.get("params"), str,
):
config["params"] = json.loads(config["params"])

configs["bundle" / relative_path] = config

Expand Down
115 changes: 115 additions & 0 deletions tests/cli/superset/sync/native/command_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ def test_native(mocker: MockerFixture, fs: FakeFilesystem) -> None:
"database_name": "GSheets",
"sqlalchemy_uri": "gsheets://",
"is_managed_externally": False,
"uuid": "uuid1",
}
dataset_config = {"table_name": "test", "is_managed_externally": False}
fs.create_file(
Expand Down Expand Up @@ -182,6 +183,7 @@ def test_native(mocker: MockerFixture, fs: FakeFilesystem) -> None:
"preset_cli.cli.superset.sync.native.command.SupersetClient",
)
client = SupersetClient()
client.get_uuids.return_value = {}
import_resources = mocker.patch(
"preset_cli.cli.superset.sync.native.command.import_resources",
)
Expand All @@ -206,6 +208,111 @@ def test_native(mocker: MockerFixture, fs: FakeFilesystem) -> None:
import_resources.assert_has_calls([mock.call(contents, client, False)])


def test_native_params_as_str(mocker: MockerFixture, fs: FakeFilesystem) -> None:
"""
Test the ``native`` command when dataset ``params`` are a string.
"""
root = Path("/path/to/root")
fs.create_dir(root)
database_config = {
"database_name": "GSheets",
"sqlalchemy_uri": "gsheets://",
"is_managed_externally": False,
"uuid": "uuid1",
}
dataset_config = {
"table_name": "test",
"is_managed_externally": False,
"params": '{"hello": "world"}',
}
fs.create_file(
root / "databases/gsheets.yaml",
contents=yaml.dump(database_config),
)
fs.create_file(
root / "datasets/gsheets/test.yaml",
contents=yaml.dump(dataset_config),
)

SupersetClient = mocker.patch(
"preset_cli.cli.superset.sync.native.command.SupersetClient",
)
client = SupersetClient()
client.get_uuids.return_value = {}
import_resources = mocker.patch(
"preset_cli.cli.superset.sync.native.command.import_resources",
)
mocker.patch("preset_cli.cli.superset.main.UsernamePasswordAuth")

runner = CliRunner()
result = runner.invoke(
superset_cli,
["https://superset.example.org/", "sync", "native", str(root)],
catch_exceptions=False,
)
assert result.exit_code == 0
contents = {
"bundle/databases/gsheets.yaml": yaml.dump(database_config),
"bundle/datasets/gsheets/test.yaml": yaml.dump(
{
"table_name": "test",
"is_managed_externally": False,
"params": {"hello": "world"},
},
),
}
import_resources.assert_has_calls([mock.call(contents, client, False)])


def test_native_password_prompt(mocker: MockerFixture, fs: FakeFilesystem) -> None:
"""
Test the ``native`` command with databases that have masked passwords.
"""
root = Path("/path/to/root")
fs.create_dir(root)
database_config = {
"database_name": "Postgres",
"sqlalchemy_uri": "postgresql://user:XXXXXXXXXX@host:5432/dbname",
"is_managed_externally": False,
"uuid": "uuid1",
}
fs.create_file(
root / "databases/gsheets.yaml",
contents=yaml.dump(database_config),
)

SupersetClient = mocker.patch(
"preset_cli.cli.superset.sync.native.command.SupersetClient",
)
client = SupersetClient()
client.get_uuids.return_value = {}
mocker.patch("preset_cli.cli.superset.sync.native.command.import_resources")
mocker.patch("preset_cli.cli.superset.main.UsernamePasswordAuth")
prompt_for_passwords = mocker.patch(
"preset_cli.cli.superset.sync.native.command.prompt_for_passwords",
)

runner = CliRunner()

result = runner.invoke(
superset_cli,
["https://superset.example.org/", "sync", "native", str(root)],
catch_exceptions=False,
)
assert result.exit_code == 0
prompt_for_passwords.assert_called()

prompt_for_passwords.reset_mock()
client.get_uuids.return_value = {"1": "uuid1"}
result = runner.invoke(
superset_cli,
["https://superset.example.org/", "sync", "native", str(root)],
catch_exceptions=False,
)
assert result.exit_code == 0
prompt_for_passwords.assert_not_called()


def test_native_load_env(
mocker: MockerFixture,
fs: FakeFilesystem,
Expand All @@ -222,6 +329,7 @@ def test_native_load_env(
"database_name": "Postgres",
"sqlalchemy_uri": '{{ env["SQLALCHEMY_URI"] }}',
"is_managed_externally": False,
"uuid": "uuid1",
}
fs.create_file(
root / "databases/postgres.yaml",
Expand All @@ -232,6 +340,7 @@ def test_native_load_env(
"preset_cli.cli.superset.sync.native.command.SupersetClient",
)
client = SupersetClient()
client.get_uuids.return_value = {}
import_resources = mocker.patch(
"preset_cli.cli.superset.sync.native.command.import_resources",
)
Expand All @@ -256,6 +365,7 @@ def test_native_load_env(
"database_name": "Postgres",
"sqlalchemy_uri": "postgres://host1",
"is_managed_externally": False,
"uuid": "uuid1",
},
),
}
Expand All @@ -273,6 +383,7 @@ def test_native_external_url(mocker: MockerFixture, fs: FakeFilesystem) -> None:
"sqlalchemy_uri": "gsheets://",
"external_url": "https://repo.example.com/databases/gsheets.yaml",
"is_managed_externally": True,
"uuid": "uuid1",
}
dataset_config = {
"table_name": "test",
Expand All @@ -292,6 +403,7 @@ def test_native_external_url(mocker: MockerFixture, fs: FakeFilesystem) -> None:
"preset_cli.cli.superset.sync.native.command.SupersetClient",
)
client = SupersetClient()
client.get_uuids.return_value = {}
import_resources = mocker.patch(
"preset_cli.cli.superset.sync.native.command.import_resources",
)
Expand Down Expand Up @@ -378,6 +490,7 @@ def test_template_in_environment(mocker: MockerFixture, fs: FakeFilesystem) -> N
"database_name": "GSheets",
"sqlalchemy_uri": "gsheets://",
"test": "{{ filepath }}",
"uuid": "uuid1",
}
fs.create_file(
root / "databases/gsheets.yaml",
Expand All @@ -388,6 +501,7 @@ def test_template_in_environment(mocker: MockerFixture, fs: FakeFilesystem) -> N
"preset_cli.cli.superset.sync.native.command.SupersetClient",
)
client = SupersetClient()
client.get_uuids.return_value = {}
import_resources = mocker.patch(
"preset_cli.cli.superset.sync.native.command.import_resources",
)
Expand All @@ -407,6 +521,7 @@ def test_template_in_environment(mocker: MockerFixture, fs: FakeFilesystem) -> N
"is_managed_externally": False,
"sqlalchemy_uri": "gsheets://",
"test": "/path/to/root/databases/gsheets.yaml",
"uuid": "uuid1",
},
),
}
Expand Down

0 comments on commit d99c9ab

Please sign in to comment.