Skip to content

Commit

Permalink
Add env vars support to Pyfunc ensembler (#181)
Browse files Browse the repository at this point in the history
* Add create router version endpoint

* Edit error message

* Update OpenAPI specs and API with new endpoint

* Add tests for CreateRouterVersion

* Add UI components for creating router versions without deployment

* Refactor UI submit event

* Add SDK method to create new router version

* Add tests for create version SDK method

* Refactor error handling

* Rename buttons

* Change button colours to shades of primary colour

* Add custom CSS colours

* Refactor variables in router view

* Refactor confirmation messages

* Update put operation to create router versions to a post operation

* Add SDK sample to reflect saving of router versions

* Remove redundant words from OpenAPI spec descriptions

* Rename UpdateSummary to RouterUpdateSummary for consistency

* Rename callback functions for clarity

* Refactor VersionComparisonView to receive two onSubmit handlers

* Simplify edit a router sample

* Refactor create_version to now be a classmethod of RouterVersion

* Refactor CreateRouterVersion endpoint to use RouterVersionConfig

* Edit UI to use refactored schema for creating router version

* Refactor create_version to now be a classmethod of RouterVersion

* Refactor create router version as router controller method

* Fix lint comments

* Reword comments in updateRouter

* Add comments in updateRouter

* Add env vars to ensembler model and deployment service

* Add pyfunc env vars to UI display

* Fix env var config display

* Add tests for env var in pyfunc_config

* Fix OpenAPI files

* Fix rebasing bugs

* Fix rebasing bugs

* Remove leftover statements

* Add migration scripts

* Fix typo in error message

* Fix typos in migration script comments

* Fix typos in migration scripts

* Fix typos in migration scripts

* Standardise migration script naming

* Fix typos in migration scripts

* Fix rebasing errors

* Refactor pyfunc config view

* Remove redundant migration scripts

* Refactor router SDK class

* Refactor pyfunc_config to prevent argument mutation
  • Loading branch information
deadlycoconuts authored Mar 23, 2022
1 parent 0cabebe commit 0763ebb
Show file tree
Hide file tree
Showing 19 changed files with 125 additions and 47 deletions.
29 changes: 29 additions & 0 deletions api/api/openapi.bundle.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1951,6 +1951,11 @@ components:
max_replica: 6
memory_request: memory_request
cpu_request: cpu_request
env:
- name: name
value: value
- name: name
value: value
timeout: timeout
updated_at: 2000-01-23T04:56:07.000+00:00
standard_config:
Expand Down Expand Up @@ -2212,6 +2217,11 @@ components:
max_replica: 6
memory_request: memory_request
cpu_request: cpu_request
env:
- name: name
value: value
- name: name
value: value
timeout: timeout
updated_at: 2000-01-23T04:56:07.000+00:00
standard_config:
Expand Down Expand Up @@ -2347,6 +2357,11 @@ components:
max_replica: 6
memory_request: memory_request
cpu_request: cpu_request
env:
- name: name
value: value
- name: name
value: value
timeout: timeout
nullable: true
properties:
Expand All @@ -2359,6 +2374,10 @@ components:
timeout:
pattern: ^[0-9]+(ms|s|m|h)$
type: string
env:
items:
$ref: '#/components/schemas/EnvVar'
type: array
required:
- ensembler_id
- project_id
Expand Down Expand Up @@ -2502,6 +2521,11 @@ components:
max_replica: 6
memory_request: memory_request
cpu_request: cpu_request
env:
- name: name
value: value
- name: name
value: value
timeout: timeout
updated_at: 2000-01-23T04:56:07.000+00:00
standard_config:
Expand Down Expand Up @@ -2634,6 +2658,11 @@ components:
max_replica: 6
memory_request: memory_request
cpu_request: cpu_request
env:
- name: name
value: value
- name: name
value: value
timeout: timeout
updated_at: 2000-01-23T04:56:07.000+00:00
standard_config:
Expand Down
6 changes: 5 additions & 1 deletion api/api/specs/routers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ info:
.timeout: &timeout
type: "string"
pattern: '^[0-9]+(ms|s|m|h)$'

paths:
"/projects/{project_id}/routers":
get:
Expand Down Expand Up @@ -851,6 +851,10 @@ components:
$ref: "#/components/schemas/ResourceRequest"
timeout:
<<: *timeout
env:
type: "array"
items:
$ref: "common.yaml#/components/schemas/EnvVar"

ResourceRequest:
type: "object"
Expand Down
2 changes: 1 addition & 1 deletion api/db-migrations/000010_add_pyfunc_config.down.sql
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
-- Removes py_func_ref_config from table.
-- Removes pyfunc_config from table.
-- Hence, this migration involves data loss for ensemblers with type that is "pyfunc"
ALTER TABLE ensembler_configs DROP COLUMN pyfunc_config;

2 changes: 1 addition & 1 deletion api/db-migrations/000010_add_pyfunc_config.up.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
-- Adds py_func_ref_config to table.
-- Adds pyfunc_config to table.
ALTER TABLE ensembler_configs ADD pyfunc_config jsonb;

2 changes: 1 addition & 1 deletion api/turing/api/request/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func (r RouterConfig) BuildRouterVersion(
}
if r.Ensembler.Type == models.EnsemblerPyFuncType {
if r.Ensembler.PyfuncConfig == nil {
return nil, errors.New("missing ensembler pyfunc reference config")
return nil, errors.New("missing ensembler pyfunc config")
}

// Verify if the ensembler given by its ProjectID and EnsemblerID exist
Expand Down
2 changes: 2 additions & 0 deletions api/turing/models/ensembler.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ type EnsemblerPyfuncConfig struct {
ResourceRequest *ResourceRequest `json:"resource_request" validate:"required"`
// Request timeout in duration format e.g. 60s
Timeout string `json:"timeout" validate:"required"`
// Environment variables to set in the container
Env EnvVars `json:"env" validate:"required"`
}

type ExperimentMapping struct {
Expand Down
2 changes: 1 addition & 1 deletion api/turing/service/router_deployment_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ func (ds *deploymentService) buildEnsemblerServiceImage(
Timeout: routerVersion.Ensembler.PyfuncConfig.Timeout,
Endpoint: PyFuncEnsemblerServiceEndpoint,
Port: PyFuncEnsemblerServicePort,
Env: models.EnvVars{},
Env: routerVersion.Ensembler.PyfuncConfig.Env,
ServiceAccount: "",
}

Expand Down
5 changes: 3 additions & 2 deletions sdk/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,13 @@ def generic_ensembler_docker_config(generic_resource_request, generic_env_var):


@pytest.fixture
def generic_ensembler_pyfunc_config(generic_resource_request):
def generic_ensembler_pyfunc_config(generic_resource_request, generic_env_var):
return turing.generated.models.EnsemblerPyfuncConfig(
project_id=77,
ensembler_id=11,
resource_request=generic_resource_request,
timeout="500ms"
timeout="500ms",
env=[generic_env_var]
)


Expand Down
19 changes: 17 additions & 2 deletions sdk/tests/router/config/router_ensembler_config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ def test_create_router_ensembler_config(id, type, standard_config, docker_config
).to_open_api()
assert actual == request.getfixturevalue(expected)


@pytest.mark.parametrize(
"project_id,ensembler_id,resource_request,timeout,expected", [
"project_id,ensembler_id,resource_request,timeout,env,expected", [
pytest.param(
77,
11,
Expand All @@ -78,6 +79,11 @@ def test_create_router_ensembler_config(id, type, standard_config, docker_config
memory_request='512Mi'
),
"500ms",
[
EnvVar(
name="env_name",
value="env_val")
],
"generic_pyfunc_router_ensembler_config"
)
])
Expand All @@ -86,6 +92,7 @@ def test_create_pyfunc_router_ensembler_config(
ensembler_id,
resource_request,
timeout,
env,
expected,
request
):
Expand All @@ -94,12 +101,13 @@ def test_create_pyfunc_router_ensembler_config(
ensembler_id=ensembler_id,
resource_request=resource_request,
timeout=timeout,
env=env
).to_open_api()
assert actual == request.getfixturevalue(expected)


@pytest.mark.parametrize(
"project_id,ensembler_id,resource_request,timeout,expected", [
"project_id,ensembler_id,resource_request,timeout,env,expected", [
pytest.param(
77,
11,
Expand All @@ -110,6 +118,11 @@ def test_create_pyfunc_router_ensembler_config(
memory_request='512Mi'
),
"500ks",
[
EnvVar(
name="env_name",
value="env_val")
],
ApiValueError
)
])
Expand All @@ -118,6 +131,7 @@ def test_create_pyfunc_router_ensembler_config_with_invalid_timeout(
ensembler_id,
resource_request,
timeout,
env,
expected
):
with pytest.raises(expected):
Expand All @@ -126,6 +140,7 @@ def test_create_pyfunc_router_ensembler_config_with_invalid_timeout(
ensembler_id=ensembler_id,
resource_request=resource_request,
timeout=timeout,
env=env
).to_open_api()


Expand Down
5 changes: 5 additions & 0 deletions sdk/turing/generated/model/ensembler_pyfunc_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
)

def lazy_import():
from turing.generated.model.env_var import EnvVar
from turing.generated.model.resource_request import ResourceRequest
globals()['EnvVar'] = EnvVar
globals()['ResourceRequest'] = ResourceRequest


Expand Down Expand Up @@ -86,6 +88,7 @@ def openapi_types():
'ensembler_id': (int,), # noqa: E501
'resource_request': (ResourceRequest,), # noqa: E501
'timeout': (str,), # noqa: E501
'env': ([EnvVar],), # noqa: E501
}

@cached_property
Expand All @@ -98,6 +101,7 @@ def discriminator():
'ensembler_id': 'ensembler_id', # noqa: E501
'resource_request': 'resource_request', # noqa: E501
'timeout': 'timeout', # noqa: E501
'env': 'env', # noqa: E501
}

_composed_schemas = {}
Expand Down Expand Up @@ -152,6 +156,7 @@ def __init__(self, project_id, ensembler_id, resource_request, timeout, *args, *
Animal class but this time we won't travel
through its discriminator because we passed in
_visited_composed_classes = (Animal,)
env ([EnvVar]): [optional] # noqa: E501
"""

_check_type = kwargs.pop('_check_type', True)
Expand Down
4 changes: 2 additions & 2 deletions sdk/turing/router/config/experiment_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ def config(self) -> Dict:

@config.setter
def config(self, config: Dict):
if config is not None and 'project_id' in config:
config['project_id'] = int(config['project_id'])
self._config = config
if self._config is not None and 'project_id' in self._config:
self.config['project_id'] = int(self._config['project_id'])

def to_open_api(self) -> OpenApiModel:
if self.config is None:
Expand Down
36 changes: 27 additions & 9 deletions sdk/turing/router/config/router_ensembler_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ def standard_config(self, standard_config: Union[turing.generated.models.Ensembl
if isinstance(standard_config, turing.generated.models.EnsemblerStandardConfig):
self._standard_config = standard_config
elif isinstance(standard_config, dict):
standard_config["experiment_mappings"] = [
openapi_standard_config = standard_config.copy()
openapi_standard_config["experiment_mappings"] = [
turing.generated.models.EnsemblerStandardConfigExperimentMappings(**mapping)
for mapping in standard_config["experiment_mappings"]
]
self._standard_config = turing.generated.models.EnsemblerStandardConfig(**standard_config)
self._standard_config = turing.generated.models.EnsemblerStandardConfig(**openapi_standard_config)
else:
self._standard_config = standard_config

Expand All @@ -79,11 +80,12 @@ def docker_config(self, docker_config: turing.generated.models.EnsemblerDockerCo
if isinstance(docker_config, turing.generated.models.EnsemblerDockerConfig):
self._docker_config = docker_config
elif isinstance(docker_config, dict):
docker_config["resource_request"] = \
turing.generated.models.ResourceRequest(**docker_config["resource_request"])
docker_config["env"] = [turing.generated.models.EnvVar(**env_var) for env_var in docker_config["env"]]
openapi_docker_config = docker_config.copy()
openapi_docker_config["resource_request"] = \
turing.generated.models.ResourceRequest(**openapi_docker_config['resource_request'])
openapi_docker_config["env"] = [turing.generated.models.EnvVar(**env_var) for env_var in docker_config['env']]
self._docker_config = turing.generated.models.EnsemblerDockerConfig(
**docker_config
**openapi_docker_config
)
else:
self._docker_config = docker_config
Expand All @@ -97,10 +99,12 @@ def pyfunc_config(self, pyfunc_config: turing.generated.models.EnsemblerPyfuncCo
if isinstance(pyfunc_config, turing.generated.models.EnsemblerPyfuncConfig):
self._pyfunc_config = pyfunc_config
elif isinstance(pyfunc_config, dict):
pyfunc_config["resource_request"] = \
openapi_pyfunc_config = pyfunc_config.copy()
openapi_pyfunc_config["resource_request"] = \
turing.generated.models.ResourceRequest(**pyfunc_config["resource_request"])
openapi_pyfunc_config["env"] = [turing.generated.models.EnvVar(**env_var) for env_var in pyfunc_config["env"]]
self._pyfunc_config = turing.generated.models.EnsemblerPyfuncConfig(
**pyfunc_config
**openapi_pyfunc_config
)
else:
self._pyfunc_config = pyfunc_config
Expand All @@ -127,19 +131,22 @@ def __init__(self,
project_id: int,
ensembler_id: int,
timeout: str,
resource_request: ResourceRequest):
resource_request: ResourceRequest,
env: List['EnvVar']):
"""
Method to create a new Pyfunc ensembler
:param project_id: project id of the current project
:param ensembler_id: ensembler_id of the ensembler
:param resource_request: ResourceRequest instance containing configs related to the resources required
:param timeout: request timeout which when exceeded, the request to the ensembler will be terminated
:param env: environment variables required by the container
"""
self.project_id = project_id
self.ensembler_id = ensembler_id
self.resource_request = resource_request
self.timeout = timeout
self.env = env
super().__init__(type="pyfunc")

@property
Expand Down Expand Up @@ -174,12 +181,23 @@ def timeout(self) -> str:
def timeout(self, timeout: str):
self._timeout = timeout

@property
def env(self) -> List['EnvVar']:
return self._env

@env.setter
def env(self, env: List['EnvVar']):
self._env = env

def to_open_api(self) -> OpenApiModel:
assert all(isinstance(env_var, EnvVar) for env_var in self.env)

self.pyfunc_config = turing.generated.models.EnsemblerPyfuncConfig(
project_id=self.project_id,
ensembler_id=self.ensembler_id,
resource_request=self.resource_request.to_open_api(),
timeout=self.timeout,
env=[env_var.to_open_api() for env_var in self.env],
)
return super().to_open_api()

Expand Down
1 change: 0 additions & 1 deletion sdk/turing/router/config/router_version.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from __future__ import annotations

import turing
import turing.generated.models
import dataclasses
from enum import Enum

Expand Down
Loading

0 comments on commit 0763ebb

Please sign in to comment.