Add env vars support to Pyfunc ensembler #181
Conversation
deadlycoconuts
force-pushed
the
add_env_vars_pyfunc_ensembler
branch
5 times, most recently
from
c442fa9 to
bf34ecb
Compare
deadlycoconuts
force-pushed
the
add_env_vars_pyfunc_ensembler
branch
from
bf34ecb to
1215fe3
Compare
| -- Migrate pyfunc_config data from old schema to new schema (with new 'env' field set as an empty array by default) | ||
| UPDATE ensembler_configs | ||
| SET pyfunc_config = (SELECT json_build_object( | ||
| 'env', '[]'::jsonb, |
There was a problem hiding this comment.
Not sure if this is really necessary since the only pyfunc ensemblers that have been created without this field are the ones in dev, but for consistency's sake I thought having a migration script for this would make things neater. Alternatively, we could consider implementing some lazy migration of the saved pyfunc ensemblers in the API?
There was a problem hiding this comment.
Actually I feel it's better to not have this in the migration, just to keep the migration history lean.
This feature is still under development. :) We don't really need to worry since we haven't cut any release with migration file # 10 (and as a consequence, we also haven't released this feature to our internal users, as you've noted). We can run this data migration manually on the internal dev DB.
| @@ -2,7 +2,7 @@ import React, { useContext } from "react"; | |||
| import { EuiDescriptionList } from "@elastic/eui"; | |||
| import EnsemblersContext from "../../../../../providers/ensemblers/context"; | |||
|
|
|||
| export const PyFuncRefConfigTable = ({ config: { ensembler_id } }) => { | |||
| export const PyFuncConfigTable = ({ config: { ensembler_id } }) => { | |||
There was a problem hiding this comment.
Fixed this for consistency
| @@ -0,0 +1,9 @@ | |||
| -- Migrate pyfunc_config data from new schema to old schema (remove the env column) | |||
There was a problem hiding this comment.
Tip: this can be simplified using the - operator like: https://stackoverflow.com/a/35283553
| -- Migrate pyfunc_config data from old schema to new schema (with new 'env' field set as an empty array by default) | ||
| UPDATE ensembler_configs | ||
| SET pyfunc_config = (SELECT json_build_object( | ||
| 'env', '[]'::jsonb, |
There was a problem hiding this comment.
Actually I feel it's better to not have this in the migration, just to keep the migration history lean.
This feature is still under development. :) We don't really need to worry since we haven't cut any release with migration file # 10 (and as a consequence, we also haven't released this feature to our internal users, as you've noted). We can run this data migration manually on the internal dev DB.
|
Thanks for the comments @krithika369 ! FYI, I'm also going to bring in the tiny little refactoring commit from the SDK minor release into this PR :) I'll be merging this if the pipeline passes 🚀 |
In PR #164, PR #165, PR #170, PR #171, and PR #174 an automated pyfunc ensembler building feature was introduced for Turing. However, the existing implementation lacked support for passing environment variables specified by the user to the running pyfunc ensembler service in its container.
This PR introduces support for the aforementioned feature, by adding an extra
envfield to theEnsemblerPyfuncConfig/pyfuncConfig/pyfunc_configstruct/model/schema used in Turing API/UI/SDK. Turing API/UI/SDK has also been correspondingly updated to allow users to specify the new field when deploying/viewing routers with pyfunc ensemblers.Additions
api/api*- Changes to the OpenAPI specs from the addition of the newenvfieldapi/turing/models/ensembler.go- Addition of the newenvfield toEnsemblerPyfuncConfigapi/turing/service/router_deployment_service.go- Slight changes to thebuildEnsemblerServiceImageto storeenvas part of anEnsemblerDockerConfigui/src/router/components/configuration/components/pyfunc_config_section/PyFuncConfigViewGroup.js- Addition of a new field to display environment variables in the configuration details panelui/src/router/components/form/components/pyfunc_config/PyFuncConfigFormGroup.js- Addition of a new field for users to input environment variables when configuring Pyfunc ensemblers as part of a router deploymentui/src/services/ensembler/PyFuncEnsembler.js- Addition of the newenvfield toPyFuncEnsemblersdk/turing/generated/model/ensembler_pyfunc_config.py- Changes to the OpenAPI autogenerated class corresponding to the pyfunc ensembler configurationsdk/turing/router/config/router_ensembler_config.py- Addition of the newenvfield topyfunc_config