From cab16eaae2aab08b2c9a70d25a1a982ad4dee9d6 Mon Sep 17 00:00:00 2001 From: badayvedat Date: Thu, 15 Aug 2024 19:34:58 +0300 Subject: [PATCH 1/7] wip: --- projects/fal/fal.yaml | 3 ++ projects/fal/pyproject.toml | 1 + projects/fal/src/fal/files.py | 87 +++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+) create mode 100644 projects/fal/fal.yaml create mode 100644 projects/fal/src/fal/files.py diff --git a/projects/fal/fal.yaml b/projects/fal/fal.yaml new file mode 100644 index 00000000..a801b11f --- /dev/null +++ b/projects/fal/fal.yaml @@ -0,0 +1,3 @@ +stable-diffusion: + loc: inference.py::diffusion + auth: shared diff --git a/projects/fal/pyproject.toml b/projects/fal/pyproject.toml index eb3d6bd0..78d28e80 100644 --- a/projects/fal/pyproject.toml +++ b/projects/fal/pyproject.toml @@ -56,6 +56,7 @@ dependencies = [ "pyjwt[crypto]>=2.8.0,<3", "uvicorn>=0.29.0,<1", "cookiecutter", + "tomli" ] [project.optional-dependencies] diff --git a/projects/fal/src/fal/files.py b/projects/fal/src/fal/files.py new file mode 100644 index 00000000..7f6eb41b --- /dev/null +++ b/projects/fal/src/fal/files.py @@ -0,0 +1,87 @@ +from functools import lru_cache +from pathlib import Path +from typing import Any, Dict, Optional, Sequence, Tuple, Union + +import tomli + + +@lru_cache +def _load_toml(path: Union[Path, str]) -> Dict[str, Any]: + with open(path, "rb") as f: + return tomli.load(f) + + +@lru_cache +def _cached_resolve(path: Path) -> Path: + return path.resolve() + + +@lru_cache +def find_project_root( + srcs: Sequence[str], stdin_filename: Optional[str] = None +) -> Tuple[Path, str]: + """Return a directory containing .git, or pyproject.toml. + + That directory will be a common parent of all files and directories + passed in `srcs`. + + If no directory in the tree contains a marker that would specify it's the + project root, the root of the file system is returned. + + Returns a two-tuple with the first element as the project root path and + the second element as a string describing the method by which the + project root was discovered. + """ + if stdin_filename is not None: + srcs = tuple(stdin_filename if s == "-" else s for s in srcs) + if not srcs: + srcs = [str(_cached_resolve(Path.cwd()))] + + path_srcs = [_cached_resolve(Path(Path.cwd(), src)) for src in srcs] + + # A list of lists of parents for each 'src'. 'src' is included as a + # "parent" of itself if it is a directory + src_parents = [ + list(path.parents) + ([path] if path.is_dir() else []) for path in path_srcs + ] + + common_base = max( + set.intersection(*(set(parents) for parents in src_parents)), + key=lambda path: path.parts, + ) + + for directory in (common_base, *common_base.parents): + print(directory) + if (directory / ".git").exists(): + return directory, ".git directory" + + if (directory / "pyproject.toml").is_file(): + pyproject_toml = _load_toml(directory / "pyproject.toml") + if "fal" in pyproject_toml.get("tool", {}): + return directory, "pyproject.toml" + + return directory, "file system root" + + +def find_pyproject_toml( + path_search_start: Tuple[str, ...], stdin_filename: Optional[str] = None +) -> Optional[str]: + """Find the absolute filepath to a pyproject.toml if it exists""" + path_project_root, _ = find_project_root(path_search_start, stdin_filename) + path_pyproject_toml = path_project_root / "pyproject.toml" + print("Root:", path_project_root) + + if path_pyproject_toml.is_file(): + return str(path_pyproject_toml) + + +def parse_pyproject_toml(path_config: str) -> Dict[str, Any]: + """Parse a pyproject toml file, pulling out relevant parts for fal. + + If parsing fails, will raise a tomli.TOMLDecodeError. + """ + pyproject_toml = _load_toml(path_config) + config: Dict[str, Any] = pyproject_toml.get("tool", {}).get("fal", {}) + config = {k.replace("--", "").replace("-", "_"): v for k, v in config.items()} + + return config From 8fb09b6454030bfb46626c33689e11fdad38348c Mon Sep 17 00:00:00 2001 From: badayvedat Date: Wed, 21 Aug 2024 16:34:38 +0300 Subject: [PATCH 2/7] feat: add fal file --- projects/fal/src/fal/cli/deploy.py | 91 ++++++++++++++++++++++++++---- projects/fal/src/fal/cli/parser.py | 18 +++--- projects/fal/src/fal/files.py | 12 +--- 3 files changed, 95 insertions(+), 26 deletions(-) diff --git a/projects/fal/src/fal/cli/deploy.py b/projects/fal/src/fal/cli/deploy.py index c5982880..37323890 100644 --- a/projects/fal/src/fal/cli/deploy.py +++ b/projects/fal/src/fal/cli/deploy.py @@ -1,6 +1,9 @@ import argparse from collections import namedtuple from pathlib import Path +from typing import Optional, Union + +from fal.files import find_pyproject_toml, parse_pyproject_toml from .parser import FalClientParser, RefAction @@ -60,11 +63,42 @@ def _get_user() -> User: raise FalServerlessError(f"Could not parse the user data: {e}") -def _deploy(args): +def _deploy_from_toml(app_name, args): + toml_path = find_pyproject_toml() + + if toml_path is None: + raise ValueError("No pyproject.toml file found.") + + fal_data = parse_pyproject_toml(toml_path) + apps = fal_data.get("apps", {}) + + try: + app_data = apps[app_name] + except KeyError: + raise ValueError(f"App {app_name} not found in pyproject.toml") + + try: + app_ref = app_data["ref"] + except KeyError: + raise ValueError(f"App {app_name} does not have a ref key in pyproject.toml") + + try: + app_auth = app_data["auth"] + except KeyError: + app_auth = "private" + + file_path, func_name = RefAction.split_ref(app_ref) + + _deploy_from_reference((file_path, func_name), app_name, app_auth, args) + + +def _deploy_from_reference( + app_ref: tuple[Optional[Union[Path, str]], ...], app_name: str, auth: str, args +): from fal.api import FalServerlessError, FalServerlessHost from fal.utils import load_function_from - file_path, func_name = args.app_ref + file_path, func_name = app_ref if file_path is None: # Try to find a python file in the current directory options = list(Path(".").glob("*.py")) @@ -77,17 +111,17 @@ def _deploy(args): ) [file_path] = options - file_path = str(file_path) + file_path = str(file_path) # type: ignore user = _get_user() host = FalServerlessHost(args.host) - isolated_function, app_name, app_auth = load_function_from( + isolated_function, guessed_app_name, app_auth = load_function_from( host, - file_path, - func_name, + file_path, # type: ignore + func_name, # type: ignore ) - app_name = args.app_name or app_name - app_auth = args.auth or app_auth or "private" + app_name = app_name or guessed_app_name # type: ignore + app_auth = auth or app_auth or "private" app_id = host.register( func=isolated_function.func, options=isolated_function.options, @@ -114,6 +148,26 @@ def _deploy(args): ) +def _is_app_name(app_ref): + is_single_file = app_ref[1] is None + is_python_file = app_ref[0].endswith(".py") + + return is_single_file and not is_python_file + + +def _deploy(args): + if _is_app_name(args.app_ref): + app_name = args.app_ref[0] + + # we do not allow --app-name and --auth to be used with app name + if args.app_name or args.auth: + raise ValueError("Cannot use --app-name or --auth with app name reference.") + + _deploy_from_toml(app_name, args) + else: + _deploy_from_reference(args.app_ref, args.app_name, args.auth, args) + + def add_parser(main_subparsers, parents): from fal.sdk import ALIAS_AUTH_MODES @@ -122,14 +176,22 @@ def valid_auth_option(option): raise argparse.ArgumentTypeError(f"{option} is not a auth option") return option - deploy_help = "Deploy a fal application." + deploy_help = ( + "Deploy a fal application. " + "If no app reference is provided, the command will look for a " + "pyproject.toml file with a [tool.fal.apps] section and deploy the " + "application specified with the provided app name." + ) + epilog = ( "Examples:\n" " fal deploy\n" " fal deploy path/to/myfile.py\n" " fal deploy path/to/myfile.py::MyApp\n" " fal deploy path/to/myfile.py::MyApp --app-name myapp --auth public\n" + " fal deploy my-app\n" ) + parser = main_subparsers.add_parser( "deploy", parents=[*parents, FalClientParser(add_help=False)], @@ -137,21 +199,30 @@ def valid_auth_option(option): help=deploy_help, epilog=epilog, ) + parser.add_argument( "app_ref", nargs="?", action=RefAction, help=( - "Application reference. " "For example: `myfile.py::MyApp`, `myfile.py`." + "Application reference. Either a file path or a file path and a " + "function name separated by '::'. If no reference is provided, the " + "command will look for a pyproject.toml file with a [tool.fal.apps] " + "section and deploy the application specified with the provided app name.\n" + "File path example: path/to/myfile.py::MyApp\n" + "App name example: my-app\n" ), ) + parser.add_argument( "--app-name", help="Application name to deploy with.", ) + parser.add_argument( "--auth", type=valid_auth_option, help="Application authentication mode (private, public).", ) + parser.set_defaults(func=_deploy) diff --git a/projects/fal/src/fal/cli/parser.py b/projects/fal/src/fal/cli/parser.py index e3b58568..378ec279 100644 --- a/projects/fal/src/fal/cli/parser.py +++ b/projects/fal/src/fal/cli/parser.py @@ -14,14 +14,18 @@ def __init__(self, *args, **kwargs): kwargs.setdefault("default", (None, None)) super().__init__(*args, **kwargs) - def __call__(self, parser, args, values, option_string=None): # noqa: ARG002 - if isinstance(values, tuple): - file_path, obj_path = values - elif values.find("::") > 1: - file_path, obj_path = values.split("::", 1) - else: - file_path, obj_path = values, None + @classmethod + def split_ref(cls, value): + if isinstance(value, tuple): + return value + + if value.find("::") > 1: + return value.split("::", 1) + return value, None + + def __call__(self, parser, args, values, option_string=None): # noqa: ARG002 + file_path, obj_path = self.split_ref(values) setattr(args, self.dest, (file_path, obj_path)) diff --git a/projects/fal/src/fal/files.py b/projects/fal/src/fal/files.py index 7f6eb41b..6361dfbf 100644 --- a/projects/fal/src/fal/files.py +++ b/projects/fal/src/fal/files.py @@ -17,9 +17,7 @@ def _cached_resolve(path: Path) -> Path: @lru_cache -def find_project_root( - srcs: Sequence[str], stdin_filename: Optional[str] = None -) -> Tuple[Path, str]: +def find_project_root(srcs: Optional[Sequence[str]]) -> Tuple[Path, str]: """Return a directory containing .git, or pyproject.toml. That directory will be a common parent of all files and directories @@ -32,8 +30,6 @@ def find_project_root( the second element as a string describing the method by which the project root was discovered. """ - if stdin_filename is not None: - srcs = tuple(stdin_filename if s == "-" else s for s in srcs) if not srcs: srcs = [str(_cached_resolve(Path.cwd()))] @@ -51,7 +47,6 @@ def find_project_root( ) for directory in (common_base, *common_base.parents): - print(directory) if (directory / ".git").exists(): return directory, ".git directory" @@ -64,12 +59,11 @@ def find_project_root( def find_pyproject_toml( - path_search_start: Tuple[str, ...], stdin_filename: Optional[str] = None + path_search_start: Optional[Tuple[str, ...]] = None, ) -> Optional[str]: """Find the absolute filepath to a pyproject.toml if it exists""" - path_project_root, _ = find_project_root(path_search_start, stdin_filename) + path_project_root, _ = find_project_root(path_search_start) path_pyproject_toml = path_project_root / "pyproject.toml" - print("Root:", path_project_root) if path_pyproject_toml.is_file(): return str(path_pyproject_toml) From 0878f0a626a6dee97528e2cdaf2a6ded28dd52e5 Mon Sep 17 00:00:00 2001 From: badayvedat Date: Wed, 21 Aug 2024 16:34:50 +0300 Subject: [PATCH 3/7] feat: add tests --- projects/fal/tests/cli/test_deploy.py | 121 ++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/projects/fal/tests/cli/test_deploy.py b/projects/fal/tests/cli/test_deploy.py index 324a1fbe..537a4efe 100644 --- a/projects/fal/tests/cli/test_deploy.py +++ b/projects/fal/tests/cli/test_deploy.py @@ -1,3 +1,7 @@ +from typing import Optional +from unittest.mock import MagicMock, patch + +import pytest from fal.cli.deploy import _deploy from fal.cli.main import parse_args @@ -6,3 +10,120 @@ def test_deploy(): args = parse_args(["deploy", "myfile.py::MyApp"]) assert args.func == _deploy assert args.app_ref == ("myfile.py", "MyApp") + + +@pytest.fixture +def mock_parse_pyproject_toml(): + return { + "apps": { + "my-app": { + "ref": "src/my_app/inference.py::MyApp", + "auth": "shared", + }, + "another-app": { + "ref": "src/another_app/run.py::AnotherApp", + "auth": "public", + }, + } + } + + +def mock_args( + app_ref: tuple[str], app_name: Optional[str] = None, auth: Optional[str] = None +): + args = MagicMock() + + args.app_ref = app_ref + args.app_name = app_name + args.auth = auth + + return args + + +@patch("fal.cli.deploy.find_pyproject_toml", return_value="pyproject.toml") +@patch("fal.cli.deploy.parse_pyproject_toml") +@patch("fal.cli.deploy._deploy_from_reference") +def test_deploy_with_toml_success( + mock_deploy_ref, mock_parse_toml, mock_find_toml, mock_parse_pyproject_toml +): + # Mocking the parse_pyproject_toml function to return a predefined dict + mock_parse_toml.return_value = mock_parse_pyproject_toml + + args = mock_args(app_ref=("my-app", None)) + + _deploy(args) + + # Ensure the correct app is deployed + mock_deploy_ref.assert_called_once_with( + ("src/my_app/inference.py", "MyApp"), "my-app", "shared", args + ) + + +@patch("fal.cli.deploy.find_pyproject_toml", return_value="pyproject.toml") +@patch("fal.cli.deploy.parse_pyproject_toml") +@patch("fal.cli.deploy._deploy_from_reference") +def test_deploy_with_toml_app_not_found( + mock_deploy_ref, mock_parse_toml, mock_find_toml, mock_parse_pyproject_toml +): + # Mocking the parse_pyproject_toml function to return a predefined dict + mock_parse_toml.return_value = mock_parse_pyproject_toml + + args = mock_args(app_ref=("non-existent-app", None)) + + # Expect a ValueError since "non-existent-app" is not in the toml + with pytest.raises( + ValueError, match="App non-existent-app not found in pyproject.toml" + ): + _deploy(args) + + +@patch("fal.cli.deploy.find_pyproject_toml", return_value="pyproject.toml") +@patch("fal.cli.deploy.parse_pyproject_toml") +@patch("fal.cli.deploy._deploy_from_reference") +def test_deploy_with_toml_missing_ref_key( + mock_deploy_ref, mock_parse_toml, mock_find_toml +): + # Mocking a toml structure without a "ref" key for a certain app + mock_parse_toml.return_value = { + "apps": { + "my-app": { + "auth": "shared", + } + } + } + + args = mock_args(app_ref=("my-app", None)) + + # Expect a ValueError since "ref" key is missing + with pytest.raises( + ValueError, match="App my-app does not have a ref key in pyproject.toml" + ): + _deploy(args) + + +@patch("fal.cli.deploy.find_pyproject_toml", return_value=None) +def test_deploy_with_toml_file_not_found(mock_find_toml): + args = mock_args(app_ref=("my-app", None)) + + # Expect a ValueError since no pyproject.toml file is found + with pytest.raises(ValueError, match="No pyproject.toml file found."): + _deploy(args) + + +@patch("fal.cli.deploy.find_pyproject_toml", return_value="pyproject.toml") +@patch("fal.cli.deploy.parse_pyproject_toml") +def test_deploy_with_toml_only_app_name_is_provided( + mock_parse_toml, mock_find_toml, mock_parse_pyproject_toml +): + mock_parse_toml.return_value = mock_parse_pyproject_toml + + args = mock_args( + app_ref=("my-app", None), app_name="custom-app-name", auth="public" + ) + + # Expect a ValueError since --app-name and --auth cannot be used with just + # the app name reference + with pytest.raises( + ValueError, match="Cannot use --app-name or --auth with app name reference." + ): + _deploy(args) From 97bdaecc6624262fa4cfbdb78aa4893c82d46c74 Mon Sep 17 00:00:00 2001 From: badayvedat Date: Wed, 21 Aug 2024 16:35:57 +0300 Subject: [PATCH 4/7] chore: remove unused file --- projects/fal/fal.yaml | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 projects/fal/fal.yaml diff --git a/projects/fal/fal.yaml b/projects/fal/fal.yaml deleted file mode 100644 index a801b11f..00000000 --- a/projects/fal/fal.yaml +++ /dev/null @@ -1,3 +0,0 @@ -stable-diffusion: - loc: inference.py::diffusion - auth: shared From 81fe919d0101d555c16da3f1b85e44f639a7cf4b Mon Sep 17 00:00:00 2001 From: badayvedat Date: Wed, 21 Aug 2024 16:36:39 +0300 Subject: [PATCH 5/7] chore: nit --- projects/fal/src/fal/cli/deploy.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/projects/fal/src/fal/cli/deploy.py b/projects/fal/src/fal/cli/deploy.py index 37323890..a1bb3cec 100644 --- a/projects/fal/src/fal/cli/deploy.py +++ b/projects/fal/src/fal/cli/deploy.py @@ -82,10 +82,7 @@ def _deploy_from_toml(app_name, args): except KeyError: raise ValueError(f"App {app_name} does not have a ref key in pyproject.toml") - try: - app_auth = app_data["auth"] - except KeyError: - app_auth = "private" + app_auth = app_data.get("auth", "private") file_path, func_name = RefAction.split_ref(app_ref) From 4c42c5d1bf431193f8a8ef19338f293e6c4a541d Mon Sep 17 00:00:00 2001 From: badayvedat Date: Wed, 21 Aug 2024 16:38:58 +0300 Subject: [PATCH 6/7] feat: add test for no auth --- projects/fal/tests/cli/test_deploy.py | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/projects/fal/tests/cli/test_deploy.py b/projects/fal/tests/cli/test_deploy.py index 537a4efe..f1e72889 100644 --- a/projects/fal/tests/cli/test_deploy.py +++ b/projects/fal/tests/cli/test_deploy.py @@ -20,9 +20,9 @@ def mock_parse_pyproject_toml(): "ref": "src/my_app/inference.py::MyApp", "auth": "shared", }, + # auth is not provided for another-app "another-app": { - "ref": "src/another_app/run.py::AnotherApp", - "auth": "public", + "ref": "src/another_app/inference.py::AnotherApp", }, } } @@ -59,6 +59,25 @@ def test_deploy_with_toml_success( ) +@patch("fal.cli.deploy.find_pyproject_toml", return_value="pyproject.toml") +@patch("fal.cli.deploy.parse_pyproject_toml") +@patch("fal.cli.deploy._deploy_from_reference") +def test_deploy_with_toml_no_auth( + mock_deploy_ref, mock_parse_toml, mock_find_toml, mock_parse_pyproject_toml +): + # Mocking the parse_pyproject_toml function to return a predefined dict + mock_parse_toml.return_value = mock_parse_pyproject_toml + + args = mock_args(app_ref=("another-app", None)) + + _deploy(args) + + # Since auth is not provided for "another-app", it should default to "private" + mock_deploy_ref.assert_called_once_with( + ("src/another_app/inference.py", "AnotherApp"), "another-app", "private", args + ) + + @patch("fal.cli.deploy.find_pyproject_toml", return_value="pyproject.toml") @patch("fal.cli.deploy.parse_pyproject_toml") @patch("fal.cli.deploy._deploy_from_reference") From df66141ca34419c1d6c8a6b897a91aeca702e742 Mon Sep 17 00:00:00 2001 From: badayvedat Date: Wed, 28 Aug 2024 13:37:33 +0300 Subject: [PATCH 7/7] revert some changes --- projects/fal/src/fal/cli/deploy.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/projects/fal/src/fal/cli/deploy.py b/projects/fal/src/fal/cli/deploy.py index 4ae89201..36d8b0b0 100644 --- a/projects/fal/src/fal/cli/deploy.py +++ b/projects/fal/src/fal/cli/deploy.py @@ -112,19 +112,20 @@ def _deploy_from_reference( user = _get_user() host = FalServerlessHost(args.host) - loaded_function = load_function_from( + loaded = load_function_from( host, file_path, # type: ignore func_name, # type: ignore ) - app_name = app_name or loaded_function.app_name # type: ignore - app_auth = auth or loaded_function.app_auth or "private" + isolated_function = loaded.function + app_name = app_name or loaded.app_name # type: ignore + app_auth = auth or loaded.app_auth or "private" app_id = host.register( - func=loaded_function.function.func, - options=loaded_function.function.options, + func=isolated_function.func, + options=isolated_function.options, application_name=app_name, application_auth_mode=app_auth, - metadata=loaded_function.function.options.host.get("metadata", {}), + metadata=isolated_function.options.host.get("metadata", {}), ) if app_id: @@ -138,12 +139,12 @@ def _deploy_from_reference( f"'{app_name}' (revision='{app_id}')." ) args.console.print("Playground:") - for endpoint in loaded_function.endpoints: + for endpoint in loaded.endpoints: args.console.print( f"\thttps://fal.ai/models/{user.username}/{app_name}{endpoint}" ) args.console.print("Endpoints:") - for endpoint in loaded_function.endpoints: + for endpoint in loaded.endpoints: args.console.print( f"\thttps://{gateway_host}/{user.username}/{app_name}{endpoint}" )