Skip to content

Commit

Permalink
misc(container): add tests for container apps and remove build_env (#375
Browse files Browse the repository at this point in the history
)

* misc(container): add tests for container apps and remove build_env

We currently do not exercise container apps in our tests, the container
app seems to be an unused fixture. This commit introduces basic tests to
ensure that container apps are built and work as expected. Furthermore,
this commit introduces a check that container args actually work.
Finally, this commit removes the `build_env` keyword from the
`ContainerImage` class. Docker has no concept of specifying environment
variables at build time except by declaring build arguments. Having that
keyword is messy and opens the door to non-hermetic builds, which is
a recipe for leaks and reproducibility problems.

Signed-off-by: squat <[email protected]>

* fix isort

---------

Signed-off-by: squat <[email protected]>
  • Loading branch information
squat authored Jan 3, 2025
1 parent 2f37295 commit 1571b28
Show file tree
Hide file tree
Showing 11 changed files with 70 additions and 16 deletions.
3 changes: 3 additions & 0 deletions projects/fal/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,8 @@ target-version = "py38"
[tool.ruff.lint]
select = ["E", "F", "W", "PLC", "PLE", "PLW", "I", "UP"]

[tool.ruff.lint.isort]
known-first-party = ["fal"]

[tool.ruff.lint.pyupgrade]
keep-runtime-typing = true
2 changes: 1 addition & 1 deletion projects/fal/src/fal/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ class ContainerImage:
from a Dockerfile.
"""

_known_keys = {"dockerfile_str", "build_env", "build_args", "registries"}
_known_keys = {"dockerfile_str", "build_args", "registries"}

@classmethod
def from_dockerfile_str(cls, text: str, **kwargs):
Expand Down
1 change: 1 addition & 0 deletions projects/fal/tests/cli/test_deploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from unittest.mock import MagicMock, patch

import pytest

from fal.cli.deploy import _deploy
from fal.cli.main import parse_args
from fal.files import find_project_root
Expand Down
1 change: 1 addition & 0 deletions projects/fal/tests/cli/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from unittest.mock import MagicMock, patch

import pytest

from fal.cli.main import parse_args
from fal.cli.run import _run
from fal.files import find_project_root
Expand Down
1 change: 1 addition & 0 deletions projects/fal/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from functools import partial

import pytest

from fal import function
from fal.flags import GRPC_HOST
from fal.sdk import get_default_credentials
Expand Down
7 changes: 4 additions & 3 deletions projects/fal/tests/integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@
from typing import Callable
from uuid import uuid4

import fal
import pytest
from pydantic import BaseModel, Field
from pydantic import __version__ as pydantic_version

import fal
from fal import FalServerlessHost, FalServerlessKeyCredentials, local, sync_dir
from fal.api import FalServerlessError, IsolatedFunction
from fal.toolkit import (
Expand All @@ -21,8 +24,6 @@
)
from fal.toolkit.file.file import CompressedFile
from fal.toolkit.utils.download_utils import _get_git_revision_hash, _hash_url
from pydantic import BaseModel, Field
from pydantic import __version__ as pydantic_version

EXAMPLE_FILE_URL = "https://raw.githubusercontent.com/fal-ai/fal/main/projects/fal/tests/assets/cat.png"

Expand Down
59 changes: 51 additions & 8 deletions projects/fal/tests/test_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@
from datetime import datetime
from typing import Generator, List, Tuple

import fal
import fal.api as api
import httpx
import pytest
from fastapi import WebSocket
from httpx import HTTPStatusError
from isolate.backends.common import active_python
from openapi_fal_rest.api.applications import app_metadata
from pydantic import BaseModel
from pydantic import __version__ as pydantic_version

import fal
import fal.api as api
from fal import apps
from fal.app import AppClient, AppClientError
from fal.cli.deploy import _get_user
Expand All @@ -19,12 +26,6 @@
from fal.exceptions._cuda import _CUDA_OOM_MESSAGE, _CUDA_OOM_STATUS_CODE
from fal.rest_client import REST_CLIENT
from fal.workflows import Workflow
from fastapi import WebSocket
from httpx import HTTPStatusError
from isolate.backends.common import active_python
from openapi_fal_rest.api.applications import app_metadata
from pydantic import BaseModel
from pydantic import __version__ as pydantic_version


class Input(BaseModel):
Expand Down Expand Up @@ -90,6 +91,26 @@ def container_addition_app(input: Input) -> Output:
return Output(result=input.lhs + input.rhs)


@fal.function(
kind="container",
image=ContainerImage.from_dockerfile_str(
f"""FROM python:{actual_python}-slim\n# {git_revision_short_hash()}
ARG OUTPUT="built incorrectly"
ENV OUTPUT="$OUTPUT"
""",
build_args={"OUTPUT": "built with build args"},
),
keep_alive=60,
machine_type="S",
serve=True,
max_concurrency=1,
)
def container_build_args_app() -> str:
import os

return os.environ["OUTPUT"]


@fal.function(
keep_alive=300,
requirements=["fastapi", "uvicorn", "pydantic==1.10.18"],
Expand Down Expand Up @@ -321,6 +342,18 @@ def test_container_app():
yield f"{user.user_id}/{app_revision}"


@pytest.fixture(scope="module")
def test_container_build_args_app():
# Create a temporary app, register it, and return the ID of it.

app_revision = container_build_args_app.host.register(
func=container_build_args_app.func,
options=container_build_args_app.options,
)
user = _get_user()
yield f"{user.user_id}/{app_revision}"


@pytest.fixture(scope="module")
def test_fastapi_app():
# Create a temporary app, register it, and return the ID of it.
Expand Down Expand Up @@ -843,3 +876,13 @@ def test_kill_runner():
assert len(runners) == 1

client.kill_runner(runners[0].runner_id)


def test_container_app_client(test_container_app: str):
response = apps.run(test_container_app, arguments={"lhs": 1, "rhs": 2})
assert response["result"] == 3


def test_container_build_args_app_client(test_container_build_args_app: str):
response = apps.run(test_container_build_args_app, {})
assert response == "built with build args"
7 changes: 4 additions & 3 deletions projects/fal/tests/test_stability.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
import subprocess
from contextlib import suppress

import fal
import pytest
from isolate.backends.common import active_python
from pydantic import __version__ as pydantic_version

import fal
from fal.api import FalServerlessError, Options
from fal.container import ContainerImage
from fal.toolkit.file import File
from isolate.backends.common import active_python
from pydantic import __version__ as pydantic_version

PACKAGE_NAME = "fall"

Expand Down
1 change: 1 addition & 0 deletions projects/fal/tests/toolkit/file_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from base64 import b64encode

import pytest

from fal.toolkit.file.file import File, GoogleStorageRepository


Expand Down
3 changes: 2 additions & 1 deletion projects/fal/tests/toolkit/image_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
from typing import Literal, overload

import pytest
from fal.toolkit import Image
from PIL import Image as PILImage
from pydantic import BaseModel, Field
from pydantic import __version__ as pydantic_version

from fal.toolkit import Image


@overload
def get_image(as_bytes: Literal[False] = False) -> PILImage.Image: ...
Expand Down
1 change: 1 addition & 0 deletions projects/fal/tests/toolkit/utils/retry.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest import mock

import pytest

from fal.toolkit.utils.retry import retry


Expand Down

0 comments on commit 1571b28

Please sign in to comment.