Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add truss patch flow tests for all python versions #205

Merged
merged 1 commit into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 25 additions & 3 deletions truss/tests/test_truss_handle.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,23 @@ def test_enable_gpu(custom_model_truss_dir_with_pre_and_post):
assert th.spec.config.resources.use_gpu


@pytest.mark.parametrize(
"python_version, expected_python_version",
[
("3.8", "py38"),
("py38", "py38"),
],
)
def test_update_python_version(
python_version,
expected_python_version,
custom_model_truss_dir_with_pre_and_post,
):
th = TrussHandle(custom_model_truss_dir_with_pre_and_post)
th.update_python_version(python_version)
assert th.spec.python_version == expected_python_version


def test_update_requirements(custom_model_truss_dir_with_pre_and_post):
th = TrussHandle(custom_model_truss_dir_with_pre_and_post)
requirements = [
Expand Down Expand Up @@ -654,11 +671,16 @@ def test_container_stuck_in_created(container_state_mock):

@pytest.mark.integration
@pytest.mark.parametrize(
"binary",
[(True), (False)],
"binary, python_version",
[
(binary, python_version)
for binary in [True, False]
for python_version in ["3.8", "3.9"]
],
)
def test_control_truss_local_update_flow(binary, custom_model_control):
def test_control_truss_local_update_flow(binary, python_version, custom_model_control):
th = TrussHandle(custom_model_control)
th.update_python_version(python_version)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not super clear on what this is doing. What is it updating based on this python_version?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This updated python version in truss_config, which in turn governs the python version on the docker image. Ultimately, this governs what python version users model code runs with.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. Does this only validate tests run in docker?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, these run with docker. What do you mean by only validate? These run the whole truss patch flow using the docker image built for truss which those versions.

Copy link
Contributor

@zero1zero zero1zero Feb 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whole truss patch flow

I see. Makes sense if this is the only to test that flow. Ideally, we want to run all tests in both environments, but agreed this is a good stop gap for now.

tag = "test-docker-custom-model-control-tag:0.0.1"

def predict_with_updated_model_code():
Expand Down
14 changes: 14 additions & 0 deletions truss/truss_handle.py
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,20 @@ def truss_hash_on_serving_container(self) -> Optional[str]:
return None
return respj["result"]

def update_python_version(self, python_version: str):
inferred_python_version = python_version
if not python_version.startswith("py"):
# support 3.9 style versions
version_parts = python_version.split(".")
inferred_python_version = f"py{version_parts[0]}{version_parts[1]}"

self._update_config(
lambda conf: replace(
conf,
python_version=inferred_python_version,
)
)

def _control_serving_container_has_partially_applied_patch(self) -> Optional[bool]:
"""Check if there is a partially applied patch on the running live_reload capable container."""
if not self.spec.live_reload:
Expand Down