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

Feat!: Remove SDK installation from our base image #443

Merged
merged 51 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
a02a7c5
remove SDK from base image
SebastianMorawiec Aug 22, 2024
35382ab
change default image used
SebastianMorawiec Aug 22, 2024
005edf7
fix cuda dockerfile
SebastianMorawiec Aug 22, 2024
fe77b8b
remove default template
SebastianMorawiec Aug 23, 2024
caa942d
fix style
SebastianMorawiec Aug 23, 2024
9f15ea8
fix integration tests
SebastianMorawiec Aug 23, 2024
580c25e
use custom WDR image
SebastianMorawiec Aug 26, 2024
f7f78f9
pass without sdk
SebastianMorawiec Aug 26, 2024
a3b748d
remove another place with sdk info
SebastianMorawiec Aug 26, 2024
bac9598
change to staging nexus
SebastianMorawiec Aug 26, 2024
d6ee436
Add pip install
SebastianMorawiec Aug 27, 2024
d7ac9a4
fix args
SebastianMorawiec Aug 27, 2024
45138e6
name change
SebastianMorawiec Aug 27, 2024
1c9128b
pip
SebastianMorawiec Aug 27, 2024
88559c1
temp stuff
SebastianMorawiec Aug 27, 2024
7f76896
add sdk everywhere
SebastianMorawiec Aug 27, 2024
2290414
try once again
SebastianMorawiec Aug 27, 2024
3f629c8
add runtime everywhere
SebastianMorawiec Aug 27, 2024
0c65a89
fix data aggregation step
SebastianMorawiec Aug 27, 2024
5d2739b
install minimum stuff
SebastianMorawiec Aug 27, 2024
dbe0d47
redo how we read sdk version installation in worker
SebastianMorawiec Aug 27, 2024
d3e2cb9
Merge remote-tracking branch 'origin/main' into Morawiec/sdk_base
SebastianMorawiec Aug 28, 2024
1db2516
remove fitering out and warning on server side
SebastianMorawiec Aug 28, 2024
a33f4bc
fix data aggregation step
SebastianMorawiec Aug 28, 2024
8ee94ce
fixes
SebastianMorawiec Aug 28, 2024
c4447b0
head_node_image has to be backwards-compatible
SebastianMorawiec Aug 29, 2024
02a6bc8
add custom image on client side
SebastianMorawiec Aug 29, 2024
d332f4c
cleanup IR
SebastianMorawiec Aug 29, 2024
5ba2dbb
remove bad code
SebastianMorawiec Aug 29, 2024
6d508d4
fix installation of workflow runtime
SebastianMorawiec Aug 29, 2024
a92fa30
fix sdk fast tests
SebastianMorawiec Aug 29, 2024
dd5f446
add gitignore for generated jsons for tests
SebastianMorawiec Aug 29, 2024
fcced82
satisfy pyright
SebastianMorawiec Aug 29, 2024
eee917a
add comment
SebastianMorawiec Aug 29, 2024
e36e25b
fix handling dev environment
SebastianMorawiec Aug 30, 2024
a350529
fix style
SebastianMorawiec Aug 30, 2024
801b8f9
filter warnings
SebastianMorawiec Aug 30, 2024
71e5831
add repo download to tests
SebastianMorawiec Aug 30, 2024
64d2208
change url
SebastianMorawiec Aug 30, 2024
f0f2fec
change to github import
SebastianMorawiec Aug 30, 2024
8771762
add subdirectory
SebastianMorawiec Aug 30, 2024
1eb9545
remove stubbed version
SebastianMorawiec Aug 30, 2024
c8cb657
fix style
SebastianMorawiec Aug 30, 2024
c25d9df
try fixing tests
SebastianMorawiec Aug 30, 2024
002ca72
style
SebastianMorawiec Aug 30, 2024
38d3edd
fix integration tests
SebastianMorawiec Aug 30, 2024
06fbecf
fix integration tests
SebastianMorawiec Sep 2, 2024
8ed835f
fix tests
SebastianMorawiec Sep 2, 2024
cef8c4b
Merge remote-tracking branch 'origin/main' into Morawiec/sdk_base
SebastianMorawiec Sep 24, 2024
0e9ab97
review adjustments
SebastianMorawiec Sep 25, 2024
83fb603
fix style
SebastianMorawiec Sep 25, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -976,15 +976,15 @@ def test_setting_resources(
None,
None,
{
"image:hub.nexus.orquestra.io/zapatacomputing/orquestra-sdk-base:worker-1.0.0a1": 1 # noqa: E501
"image:hub.stage.nexus.orquestra.io/zapatacomputing/orquestra-sdk-base:worker-1.0.0a1": 1 # noqa: E501
},
{},
),
(
None,
1,
{
"image:hub.nexus.orquestra.io/zapatacomputing/orquestra-sdk-base:worker-1.0.0a1-cuda": 1 # noqa: E501
"image:hub.stage.nexus.orquestra.io/zapatacomputing/orquestra-sdk-base:worker-1.0.0a1-cuda": 1 # noqa: E501
},
{
"num_gpus": 1,
Expand Down Expand Up @@ -1057,8 +1057,9 @@ def test_with_env_not_set(
# Then
calls = client.add_options.call_args_list

# We should only have two calls: our invocation and the aggregation step
assert len(calls) == 2
# We should only have three calls:
# invocation aggregation step and error handling for aggregation step
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the 3rd call?

assert len(calls) == 3
# Checking our call did not have any resources included
assert calls[0] == call(
ANY,
Expand Down
23 changes: 20 additions & 3 deletions projects/orquestra-sdk/tests/sdk/driver/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"""
Tests for orquestra.sdk._base._driver._client.
"""
import warnings
from datetime import timedelta
from typing import Any, Dict, List, Optional
from unittest.mock import Mock, create_autospec
Expand All @@ -12,11 +13,12 @@
import pytest
import responses
from orquestra.workflow_shared._spaces._structs import ProjectRef
from orquestra.workflow_shared.schema.ir import WorkflowDef
from orquestra.workflow_shared.schema.ir import Version, WorkflowDef
from orquestra.workflow_shared.schema.responses import JSONResult, PickleResult
from orquestra.workflow_shared.schema.workflow_run import RunStatus, State, TaskRun

import orquestra.sdk as sdk
from orquestra.sdk._client._base import _traversal
from orquestra.sdk._client._base._driver import _exceptions
from orquestra.sdk._client._base._driver._client import (
DriverClient,
Expand Down Expand Up @@ -132,21 +134,35 @@ def task_run_id(self):
def task_inv_id(self):
return "00000000-0000-0000-0000-000000000000"

@pytest.fixture
def mask_sdk_version(self, monkeypatch):
monkeypatch.setattr(
_traversal,
"get_current_sdk_version",
lambda *_: Version(
original="0.66.0", major=0, minor=66, patch=0, is_prerelease=False
),
)

@pytest.fixture
def status(self):
return RunStatus(state=State.SUCCEEDED, start_time=None, end_time=None)

@pytest.fixture
def workflow_def(self):
@sdk.task
@sdk.task(custom_image="")
def task():
return 1

@sdk.workflow
def workflow():
return task()

return workflow().model
with warnings.catch_warnings():
warnings.filterwarnings(
"ignore", "Attempting to read a workflow definition"
)
return workflow().model

class TestWorkflowDefinitions:
# ------ queries ------
Expand Down Expand Up @@ -454,6 +470,7 @@ def endpoint_mocker(endpoint_mocker_base, base_uri: str):
],
)
def test_params_encoding(
mask_sdk_version,
endpoint_mocker,
client: DriverClient,
workflow_def_id,
Expand Down
12 changes: 12 additions & 0 deletions projects/orquestra-sdk/tests/sdk/test_traversal.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,18 @@
wf_pass_obj_with_num_from_task,
)

pytestmark = pytest.mark.filterwarnings(
"ignore::pytest.PytestUnraisableExceptionWarning",
"ignore::orquestra.workflow_shared.exceptions.VersionMismatch",
)


@pytest.fixture(autouse=True)
def mask_sdk_version(monkeypatch):
mocked_installed_version = Mock(return_value="0.66.0")
monkeypatch.setattr(_versions, "get_installed_version", mocked_installed_version)
monkeypatch.setattr(git.Repo, "is_dirty", Mock(return_value=False))


# --- Tasks used in tests --- #
@_dsl.task()
Expand Down
Loading