Skip to content

Commit

Permalink
pr into #1384 (#1407)
Browse files Browse the repository at this point in the history
Signed-off-by: Yee Hing Tong <[email protected]>
Adds a test for false SSL for the old .ini style of config
Adds a shortcut for the pyflyte run/register commands - if no files are detected at all, create for sandbox instead. I think this is okay. The other option would be to fall back to checking the ~/.flyte/config-sandbox.yml file but I think using the code option is a bit cleaner, even though it ties us closer to the flytectl demo port settings.
  • Loading branch information
wild-endeavor authored Jan 10, 2023
1 parent 3f3c3d5 commit bfdefd3
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 10 deletions.
15 changes: 10 additions & 5 deletions flytekit/clis/sdk_in_container/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import click

from flytekit.clis.sdk_in_container.constants import CTX_CONFIG_FILE
from flytekit.configuration import Config, ImageConfig
from flytekit.configuration import Config, ImageConfig, get_config_file
from flytekit.loggers import cli_logger
from flytekit.remote.remote import FlyteRemote

Expand All @@ -25,10 +25,15 @@ def get_and_save_remote_with_click_context(
:return: FlyteRemote instance
"""
cfg_file_location = ctx.obj.get(CTX_CONFIG_FILE)
cfg_obj = Config.auto(cfg_file_location)
cli_logger.info(
f"Creating remote with config {cfg_obj}" + (f" with file {cfg_file_location}" if cfg_file_location else "")
)
cfg_file = get_config_file(cfg_file_location)
if cfg_file is None:
cfg_obj = Config.for_sandbox()
cli_logger.info(f"No config files found, creating remote with sandbox config")
else:
cfg_obj = Config.auto(cfg_file_location)
cli_logger.info(
f"Creating remote with config {cfg_obj}" + (f" with file {cfg_file_location}" if cfg_file_location else "")
)
r = FlyteRemote(cfg_obj, default_project=project, default_domain=domain)
if save:
ctx.obj[FLYTE_REMOTE_INSTANCE_KEY] = r
Expand Down
6 changes: 3 additions & 3 deletions flytekit/configuration/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ class PlatformConfig(object):
"""

endpoint: str = "localhost:30080"
insecure: bool = True
insecure: bool = False
insecure_skip_verify: bool = False
console_endpoint: typing.Optional[str] = None
command: typing.Optional[typing.List[str]] = None
Expand Down Expand Up @@ -529,7 +529,7 @@ def with_params(
)

@classmethod
def auto(cls, config_file: typing.Union[str, ConfigFile] = None) -> Config:
def auto(cls, config_file: typing.Union[str, ConfigFile, None] = None) -> Config:
"""
Automatically constructs the Config Object. The order of precedence is as follows
1. first try to find any env vars that match the config vars specified in the FLYTE_CONFIG format.
Expand Down Expand Up @@ -560,7 +560,7 @@ def for_sandbox(cls) -> Config:
return Config(
platform=PlatformConfig(endpoint="localhost:30080", auth_mode="Pkce", insecure=True),
data_config=DataConfig(
s3=S3Config(endpoint="http://localhost:30084", access_key_id="minio", secret_access_key="miniostorage")
s3=S3Config(endpoint="http://localhost:30002", access_key_id="minio", secret_access_key="miniostorage")
),
)

Expand Down
2 changes: 2 additions & 0 deletions tests/flytekit/unit/cli/pyflyte/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from flytekit.clients.friendly import SynchronousFlyteClient
from flytekit.clis.sdk_in_container import pyflyte
from flytekit.clis.sdk_in_container.helpers import get_and_save_remote_with_click_context
from flytekit.configuration import Config
from flytekit.core import context_manager
from flytekit.remote.remote import FlyteRemote

Expand All @@ -34,6 +35,7 @@ def test_saving_remote(mock_remote):
mock_context.obj = {}
get_and_save_remote_with_click_context(mock_context, "p", "d")
assert mock_context.obj["flyte_remote"] is not None
mock_remote.assert_called_once_with(Config.for_sandbox(), default_project="p", default_domain="d")


def test_register_with_no_package_or_module_argument():
Expand Down
2 changes: 1 addition & 1 deletion tests/flytekit/unit/configuration/configs/good.config
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ assumable_iam_role=some_role


[platform]

url=fakeflyte.com
insecure=false

[madeup]

Expand Down
4 changes: 4 additions & 0 deletions tests/flytekit/unit/configuration/configs/nossl.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
admin:
endpoint: dns:///flyte.mycorp.io
authType: Pkce
insecure: false
7 changes: 7 additions & 0 deletions tests/flytekit/unit/configuration/test_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from flytekit.configuration import ConfigEntry, get_config_file, set_if_exists
from flytekit.configuration.file import LegacyConfigEntry, _exists
from flytekit.configuration.internal import Platform


def test_set_if_exists():
Expand Down Expand Up @@ -137,3 +138,9 @@ def test_env_var_bool_transformer(mock_file_read):

# The last read should've triggered the file read since now the env var is no longer set.
assert mock_file_read.call_count == 1


def test_use_ssl():
config_file = get_config_file(os.path.join(os.path.dirname(os.path.realpath(__file__)), "configs/good.config"))
res = Platform.INSECURE.read(config_file)
assert res is False
2 changes: 1 addition & 1 deletion tests/flytekit/unit/configuration/test_internal.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,4 +82,4 @@ def test_some_int(mocked):
def test_default_platform_config_endpoint_insecure():
platform_config = PlatformConfig()
assert platform_config.endpoint == "localhost:30080"
assert platform_config.insecure
assert platform_config.insecure is False
6 changes: 6 additions & 0 deletions tests/flytekit/unit/configuration/test_yaml_file.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,9 @@ def test_real_config():

res = Credentials.SCOPES.read(config_file)
assert res == ["all"]


def test_use_ssl():
config_file = get_config_file(os.path.join(os.path.dirname(os.path.realpath(__file__)), "configs/nossl.yaml"))
res = Platform.INSECURE.read(config_file)
assert res is False

0 comments on commit bfdefd3

Please sign in to comment.