Update default config to work out-of-the-box with flytectl demo (#1384)

Signed-off-by: Niels Bantilan <niels.bantilan@gmail.com>

flytekit/clis/sdk_in_container/helpers.py

@@ -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
 
@@ -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("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

flytekit/configuration/__init__.py

@@ -300,7 +300,7 @@ class PlatformConfig(object):
     :param endpoint: DNS for Flyte backend
     :param insecure: Whether or not to use SSL
     :param insecure_skip_verify: Wether to skip SSL certificate verification
-    :param console_endpoint: endpoint for console if differenet than Flyte backend
+    :param console_endpoint: endpoint for console if different than Flyte backend
     :param command: This command is executed to return a token using an external process.
     :param client_id: This is the public identifier for the app which handles authorization for a Flyte deployment.
       More details here: https://www.oauth.com/oauth2-servers/client-registration/client-id-secret/.
@@ -311,7 +311,7 @@ class PlatformConfig(object):
     :param auth_mode: The OAuth mode to use. Defaults to pkce flow.
     """
 
-    endpoint: str = "localhost:30081"
+    endpoint: str = "localhost:30080"
     insecure: bool = False
     insecure_skip_verify: bool = False
     console_endpoint: typing.Optional[str] = None
@@ -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.
@@ -558,9 +558,9 @@ def for_sandbox(cls) -> Config:
         :return: Config
         """
         return Config(
-            platform=PlatformConfig(endpoint="localhost:30081", auth_mode="Pkce", insecure=True),
+            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")
             ),
         )
 

flytekit/configuration/file.py

@@ -18,6 +18,11 @@
 FLYTECTL_CONFIG_ENV_VAR = "FLYTECTL_CONFIG"
 
 
+def _exists(val: typing.Any) -> bool:
+    """Check if a value is defined."""
+    return isinstance(val, bool) or bool(val is not None and val)
+
+
 @dataclass
 class LegacyConfigEntry(object):
     """
@@ -63,7 +68,7 @@ def read_from_file(
 @dataclass
 class YamlConfigEntry(object):
     """
-    Creates a record for the config entry. contains
+    Creates a record for the config entry.
     Args:
         switch: dot-delimited string that should match flytectl args. Leaving it as dot-delimited instead of a list
           of strings because it's easier to maintain alignment with flytectl.
@@ -80,10 +85,11 @@ def read_from_file(
             return None
         try:
             v = cfg.get(self)
-            if v:
+            if _exists(v):
                 return transform(v) if transform else v
         except Exception:
             ...
+
         return None
 
 
@@ -273,7 +279,7 @@ def set_if_exists(d: dict, k: str, v: typing.Any) -> dict:
 
         The input dictionary ``d`` will be mutated.
     """
-    if v:
+    if _exists(v):
         d[k] = v
     return d
 

tests/flytekit/unit/cli/pyflyte/test_register.py

@@ -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
 
@@ -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():

tests/flytekit/unit/configuration/configs/good.config

@@ -7,8 +7,8 @@ assumable_iam_role=some_role
 
 
 [platform]
-
 url=fakeflyte.com
+insecure=false
 
 [madeup]
 

tests/flytekit/unit/configuration/configs/nossl.yaml

@@ -0,0 +1,4 @@
+admin:
+  endpoint: dns:///flyte.mycorp.io
+  authType: Pkce
+  insecure: false

tests/flytekit/unit/configuration/test_file.py

@@ -7,7 +7,8 @@
 from pytimeparse.timeparse import timeparse
 
 from flytekit.configuration import ConfigEntry, get_config_file, set_if_exists
-from flytekit.configuration.file import LegacyConfigEntry
+from flytekit.configuration.file import LegacyConfigEntry, _exists
+from flytekit.configuration.internal import Platform
 
 
 def test_set_if_exists():
@@ -21,6 +22,25 @@ def test_set_if_exists():
     assert d["k"] == "x"
 
 
+@pytest.mark.parametrize(
+    "data, expected",
+    [
+        [1, True],
+        [1.0, True],
+        ["foo", True],
+        [True, True],
+        [False, True],
+        [[1], True],
+        [{"k": "v"}, True],
+        [None, False],
+        [[], False],
+        [{}, False],
+    ],
+)
+def test_exists(data, expected):
+    assert _exists(data) is expected
+
+
 def test_get_config_file():
     c = get_config_file(None)
     assert c is None
@@ -118,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

tests/flytekit/unit/configuration/test_internal.py

@@ -77,3 +77,9 @@ def test_some_int(mocked):
     res = AWS.RETRIES.read(cfg)
     assert type(res) is int
     assert res == 5
+
+
+def test_default_platform_config_endpoint_insecure():
+    platform_config = PlatformConfig()
+    assert platform_config.endpoint == "localhost:30080"
+    assert platform_config.insecure is False

tests/flytekit/unit/configuration/test_yaml_file.py

@@ -14,6 +14,7 @@ def test_config_entry_file():
     assert c.read() is None
 
     cfg = get_config_file(os.path.join(os.path.dirname(os.path.realpath(__file__)), "configs/sample.yaml"))
+    assert cfg.yaml_config is not None
     assert c.read(cfg) == "flyte.mycorp.io"
 
     c = ConfigEntry(LegacyConfigEntry("platform", "url2", str))  # Does not exist
@@ -26,6 +27,7 @@ def test_config_entry_file_normal():
     cfg = get_config_file(os.path.join(os.path.dirname(os.path.realpath(__file__)), "configs/no_images.yaml"))
     images_dict = Images.get_specified_images(cfg)
     assert images_dict == {}
+    assert cfg.yaml_config is not None
 
 
 @mock.patch("flytekit.configuration.file.getenv")
@@ -43,6 +45,7 @@ def test_config_entry_file_2(mock_get):
 
     cfg = get_config_file(sample_yaml_file_name)
     assert c.read(cfg) == "flyte.mycorp.io"
+    assert cfg.yaml_config is not None
 
     c = ConfigEntry(LegacyConfigEntry("platform", "url2", str))  # Does not exist
     assert c.read(cfg) is None
@@ -67,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

tests/flytekit/unit/remote/test_remote.py

@@ -231,7 +231,7 @@ def test_generate_console_http_domain_sandbox_rewrite(mock_client):
         remote = FlyteRemote(
             config=Config.auto(config_file=temp_filename), default_project="project", default_domain="domain"
         )
-        assert remote.generate_console_http_domain() == "http://localhost:30080"
+        assert remote.generate_console_http_domain() == "http://localhost:30081"
 
         with open(temp_filename, "w") as f:
             # This string is similar to the relevant configuration emitted by flytectl in the cases of both demo and sandbox.