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.

Sign up for GitHub

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

Jump to bottom

Configure Turing UI to use Standard Ensembler for Custom Experiment Engines #235

Merged
merged 16 commits into from
Sep 27, 2022
Merged

Configure Turing UI to use Standard Ensembler for Custom Experiment Engines #235

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
87f8e1a
Update type ensembler options to allow std ensemblers for custom exp …
deadlycoconuts Sep 9, 2022
cdaceec
Make create and edit router consume standard ensembler component from…
deadlycoconuts Sep 12, 2022
2ba195b
Add route name path to standard ensembler config struct
deadlycoconuts Sep 13, 2022
65a184c
Clean up props to be passed to remote ensembler component
deadlycoconuts Sep 14, 2022
f42ba35
Add routes prop to pass to the remote standard ensembler
deadlycoconuts Sep 14, 2022
6b1416e
Update typeOptions to return correct variant of standard ensembler
deadlycoconuts Sep 20, 2022
d151b10
Add standard ensembler config view
deadlycoconuts Sep 20, 2022
f26e615
Rename prop for StandardEnsemblerLoaderComponent
deadlycoconuts Sep 21, 2022
04e5faa
Make status colours use default EUI palette as much as possible
deadlycoconuts Sep 22, 2022
5a3a938
Remove redundant empty tags
deadlycoconuts Sep 23, 2022
620adc1
Refactor components to use common RemoteLoaderComponent
deadlycoconuts Sep 23, 2022
95faa74
Replace colour hexes corresponding to subdued to default
deadlycoconuts Sep 23, 2022
5f1cf84
Add docs for standard ensembler
deadlycoconuts Sep 26, 2022
4478b71
Remove redundant description for std ensembler and custom exp engines
deadlycoconuts Sep 26, 2022
572fe8d
Rename generic component loader as ExperimentEngineComponentLoader
deadlycoconuts Sep 26, 2022
d5ad99b
Remove redundant line from docs
deadlycoconuts Sep 26, 2022
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
21 changes: 11 additions & 10 deletions ...iments/ExperimentEngineLoaderComponent.js → ...remote_component/RemoteLoaderComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ const LoadDynamicScript = ({ url, setReady, setFailed }) => {
};

// Dynamic Script Loading component wrapper
export const ExperimentEngineLoaderComponent = ({
export const RemoteLoaderComponent = ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: I think it's ok to keep this component as it was, except may be make the componentName configurable. After all, the std ensembler UI is tied to the experiment engine and it's the engine whose configs we are loading.

If it helps, we can rename it from ExperimentEngineLoaderComponent -> ExperimentEngineComponentLoader :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait so do you mean we should keep both components separate, i.e. StandardEnsemblerLoaderComponent and ExperimentEngineLoaderComponent, or combining them as @terryyylim had suggested? 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just noticed the older comment. I do think it makes sense to combine the way you have it. But I don't think this should be called a generic RemoteLoaderComponent because the logic inside is still looking at getting some url, configs (which are coupled to the experiment engine). So, the implementation can remain as is, but I think it still makes sense to call it something related to experiment engine - hence my suggestion is to use ExperimentEngineComponentLoader as the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok! I've just renamed the component as you have suggested!

FallbackView,
experimentEngine,
remoteUi,
componentName,
children,
}) => {
const [urlReady, setUrlReady] = useState(false);
Expand All @@ -28,30 +29,30 @@ export const ExperimentEngineLoaderComponent = ({
const [configFailed, setConfigFailed] = useState(false);

return urlFailed ? (
<FallbackView text={"Failed to load Experiment Engine"} />
<FallbackView text={`Failed to load ${componentName}`} />
) : configFailed ? (
<FallbackView text={"Failed to load Experiment Engine Config"} />
<FallbackView text={`Failed to load ${componentName} Config`} />
) : !urlReady || !configReady ? (
<>
{!!experimentEngine.url && !urlReady && (
{!!remoteUi.url && !urlReady && (
<LoadDynamicScript
setReady={setUrlReady}
setFailed={setUrlFailed}
url={experimentEngine.url}
url={remoteUi.url}
/>
)}
{!!experimentEngine.config && !configReady && (
{!!remoteUi.config && !configReady && (
<LoadDynamicScript
setReady={setConfigReady}
setFailed={setConfigFailed}
url={experimentEngine.config}
url={remoteUi.config}
/>
)}
<FallbackView text={"Loading Experiment Engine..."} />
<FallbackView text={`Loading ${componentName}...`} />
</>
) : (
children
);
};

export default ExperimentEngineLoaderComponent;
export default RemoteLoaderComponent;
10 changes: 6 additions & 4 deletions ui/src/experiment/ExperimentsRouter.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
EuiPageTemplate,
} from "@elastic/eui";
import { RemoteComponent } from "../components/remote_component/RemoteComponent";
import { ExperimentEngineLoaderComponent } from "../components/experiments/ExperimentEngineLoaderComponent";
import RemoteLoaderComponent from "../components/remote_component/RemoteLoaderComponent";

import { useConfig } from "../config";

Expand Down Expand Up @@ -32,16 +32,18 @@ const RemoteRouter = ({ projectId }) => {
return (
<React.Suspense
fallback={<FallbackView text="Loading Experiment Engine config" />}>
<ExperimentEngineLoaderComponent
<RemoteLoaderComponent
FallbackView={FallbackView}
experimentEngine={defaultExperimentEngine}>
remoteUi={defaultExperimentEngine}
componentName="Experiment Engine"
>
<RemoteComponent
scope={defaultExperimentEngine.name}
name="./ExperimentsLandingPage"
fallback={<FallbackView text="Loading Experiment Engine" />}
projectId={projectId}
/>
</ExperimentEngineLoaderComponent>
</RemoteLoaderComponent>
</React.Suspense>
);
};
Expand Down
28 changes: 9 additions & 19 deletions ui/src/router/components/configuration/components/EnsemblerConfigSection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import React, { Fragment } from "react";
import { DockerConfigViewGroup } from "./docker_config_section/DockerConfigViewGroup";
import { FallbackRouteConfigSection } from "./standard_config_section/FallbackRouteConfigSection";
import { TreatmentMappingConfigSection } from "./standard_config_section/TreatmentMappingConfigSection";
import { ExperimentEngineContextProvider } from "../../../../providers/experiments/ExperimentEngineContextProvider";
import { NopConfigViewGroup } from "./nop_config_section/NopConfigViewGroup";
import { PyFuncConfigViewGroup } from "./pyfunc_config_section/PyFuncConfigViewGroup";
import { StandardConfigViewGroup } from "./standard_config_section/StandardConfigViewGroup"
import { EnsemblersContextContextProvider } from "../../../../providers/ensemblers/context";
import { EuiFlexGroup, EuiFlexItem } from "@elastic/eui";

export const EnsemblerConfigSection = ({
projectId,
config: {
routes,
ensembler,
experiment_engine: { type, config: experimentConfig },
},
Expand Down Expand Up @@ -38,22 +37,13 @@ export const EnsemblerConfigSection = ({
)}
{ensembler.type === "standard" && (
<ExperimentEngineContextProvider>
<EuiFlexGroup direction="column">
<EuiFlexItem>
<TreatmentMappingConfigSection
engine={type}
experiments={(experimentConfig || {}).experiments || []}
mappings={ensembler.standard_config.experiment_mappings}
/>
</EuiFlexItem>
<EuiFlexItem>
<FallbackRouteConfigSection
fallbackResponseRouteId={
ensembler.standard_config.fallback_response_route_id
}
/>
</EuiFlexItem>
</EuiFlexGroup>
<StandardConfigViewGroup
projectId={projectId}
routes={routes}
standardConfig={ensembler.standard_config}
experimentConfig={(experimentConfig || {}).experiments || []}
type={type}
/>
</ExperimentEngineContextProvider>
)}
</Fragment>
Expand Down
10 changes: 6 additions & 4 deletions ui/src/router/components/configuration/components/ExperimentConfigSection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiPanel } from "@elastic/eui";

import { ConfigSectionPanel } from "../../../../components/config_section";
import { RemoteComponent } from "../../../../components/remote_component/RemoteComponent";
import { ExperimentEngineLoaderComponent } from "../../../../components/experiments/ExperimentEngineLoaderComponent";
import RemoteLoaderComponent from "../../../../components/remote_component/RemoteLoaderComponent";
import ExperimentEngineContext from "../../../../providers/experiments/context";

import { StandardExperimentConfigGroup } from "./experiment_config_section/StandardExperimentConfigGroup";
Expand All @@ -29,17 +29,19 @@ const CustomExperimentConfigView = ({ projectId, remoteUi, config }) => {
return (
<React.Suspense
fallback={<FallbackView text="Loading Experiment Engine config" />}>
<ExperimentEngineLoaderComponent
<RemoteLoaderComponent
FallbackView={FallbackView}
experimentEngine={remoteUi}>
remoteUi={remoteUi}
componentName="Experiment Engine"
>
<RemoteComponent
scope={remoteUi.name}
name="./ExperimentEngineConfigDetails"
fallback={<FallbackView text="Loading Experiment Engine config" />}
projectId={projectId}
config={config}
/>
</ExperimentEngineLoaderComponent>
</RemoteLoaderComponent>
</React.Suspense>
);
};
Expand Down
95 changes: 95 additions & 0 deletions ...er/components/configuration/components/standard_config_section/StandardConfigViewGroup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
import React, { useContext } from "react";

import { EuiFlexGroup, EuiFlexItem, EuiSpacer } from "@elastic/eui";
import { TreatmentMappingConfigSection } from "./TreatmentMappingConfigSection";
import { FallbackRouteConfigSection } from "./FallbackRouteConfigSection";
import ExperimentEngineContext from "../../../../../providers/experiments/context";
import { Panel } from "../../../form/components/Panel";
import RemoteLoaderComponent from "../../../../../components/remote_component/RemoteLoaderComponent";
import { RemoteComponent } from "../../../../../components/remote_component/RemoteComponent";

const FallbackView = ({ text }) => (
<EuiFlexItem grow={true}>
<Panel title="Route Selection">
<EuiSpacer size="m" />
{text}
</Panel>
</EuiFlexItem>
);

const StandardEnsemblerWithCustomExperimentEngineConfigView = ({
remoteUi,
projectId,
routes,
routeNamePath
}) => {
// Load component from remote host
return (
<React.Suspense
fallback={<FallbackView text="Loading Standard Ensembler config for the selected Custom Experiment Engine" />}>
<RemoteLoaderComponent
FallbackView={FallbackView}
remoteUi={remoteUi}
componentName="Standard Ensembler"
>
<RemoteComponent
scope={remoteUi.name}
name="./StandardEnsemblerConfigDetails"
fallback={<FallbackView text="Loading Standard Ensembler config for the selected Custom Experiment Engine" />}
projectId={projectId}
routes={routes}
routeNamePath={routeNamePath}
/>
</RemoteLoaderComponent>
</React.Suspense>
);
};

export const StandardConfigViewGroup = ({
projectId,
routes,
standardConfig,
experimentConfig,
type
}) => {
const { getEngineProperties, isLoaded } = useContext(ExperimentEngineContext);

const engineProps = getEngineProperties(type);

return (
!!standardConfig && (
<EuiFlexGroup direction="column">
{engineProps?.type === "standard" ? (
<EuiFlexItem>
<TreatmentMappingConfigSection
engine={type}
experiments={experimentConfig}
mappings={standardConfig.experiment_mappings}
/>
</EuiFlexItem>
) : isLoaded ? (
<EuiFlexItem>
<StandardEnsemblerWithCustomExperimentEngineConfigView
remoteUi={engineProps.custom_experiment_manager_config.remote_ui}
projectId={projectId}
routes={routes}
routeNamePath={standardConfig.route_name_path}
/>
</EuiFlexItem>
) : (
<EuiFlexItem>
<FallbackView text={"Loading ..."} />
</EuiFlexItem>
)}

<EuiFlexItem>
<FallbackRouteConfigSection
fallbackResponseRouteId={
standardConfig.fallback_response_route_id
}
/>
</EuiFlexItem>
</EuiFlexGroup>
)
);
};
92 changes: 80 additions & 12 deletions ...ponents/form/components/ensembler_config/standard_ensembler/StandardEnsemblerFormGroup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,61 @@
import React, { useEffect } from "react";
import { EuiFlexItem, EuiText } from "@elastic/eui";
import React, { useContext, useEffect } from "react";
import { EuiFlexItem, EuiSpacer, EuiText } from "@elastic/eui";

import { get } from "../../../../../../components/form/utils";
import { StandardEnsembler } from "../../../../../../services/ensembler";
import { StandardEnsemblerPanel } from "./StandardEnsemblerPanel";
import { RouteSelectionPanel } from "../RouteSelectionPanel";
import { FormLabelWithToolTip } from "../../../../../../components/form/label_with_tooltip/FormLabelWithToolTip";
import { useOnChangeHandler } from "../../../../../../components/form/hooks/useOnChangeHandler";
import ExperimentEngineContext from "../../../../../../providers/experiments/context";
import { Panel } from "../../Panel";
import { RemoteComponent } from "../../../../../../components/remote_component/RemoteComponent";
import RemoteLoaderComponent from "../../../../../../components/remote_component/RemoteLoaderComponent";

const FallbackView = ({ text }) => (
<EuiFlexItem grow={true}>
<Panel title="Route Selection">
<EuiSpacer size="m" />
{text}
</Panel>
</EuiFlexItem>
);

const StandardEnsemblerWithCustomExperimentEnginePanel = ({
remoteUi,
projectId,
routes,
routeNamePath,
onChange,
errors,
}) => {
// Load component from remote host
return (
<React.Suspense
fallback={<FallbackView text="Loading Standard Ensembler config for the selected Custom Experiment Engine" />}>
<RemoteLoaderComponent
FallbackView={FallbackView}
remoteUi={remoteUi}
componentName="Standard Ensembler"
>
<RemoteComponent
scope={remoteUi.name}
name="./EditStandardEnsemblerConfig"
fallback={<FallbackView text="Loading Standard Ensembler form for the selected Custom Experiment Engine" />}
projectId={projectId}
routes={routes}
routeNamePath={routeNamePath}
onChange={onChange}
errors={errors}
/>
</RemoteLoaderComponent>
</React.Suspense>
);
};

export const StandardEnsemblerFormGroup = ({
experimentConfig = {},
projectId,
experimentEngine = {},
routes,
rules,
default_traffic_rule,
Expand All @@ -19,10 +65,14 @@ export const StandardEnsemblerFormGroup = ({
}) => {
const { onChange } = useOnChangeHandler(onChangeHandler);

const { getEngineProperties, isLoaded } = useContext(ExperimentEngineContext);

useEffect(() => {
!standardConfig && onChangeHandler(StandardEnsembler.newConfig());
}, [standardConfig, onChangeHandler]);

const engineProps = getEngineProperties(experimentEngine.type);

const routeOptions = [
{
value: "nop",
Expand All @@ -47,15 +97,33 @@ export const StandardEnsemblerFormGroup = ({
return (
!!standardConfig && (
<>
<EuiFlexItem>
<StandardEnsemblerPanel
experiments={experimentConfig.experiments}
mappings={standardConfig.experiment_mappings}
routeOptions={routeOptions}
onChangeHandler={onChange("experiment_mappings")}
errors={get(errors, "experiment_mappings")}
/>
</EuiFlexItem>
{engineProps?.type === "standard" ? (
<EuiFlexItem>
<StandardEnsemblerPanel
experiments={experimentEngine.config.experiments}
mappings={standardConfig.experiment_mappings}
routeOptions={routeOptions}
onChangeHandler={onChange("experiment_mappings")}
errors={get(errors, "experiment_mappings")}
/>
</EuiFlexItem>
) : isLoaded ? (
<EuiFlexItem>
<StandardEnsemblerWithCustomExperimentEnginePanel
remoteUi={engineProps.custom_experiment_manager_config.remote_ui}
projectId={projectId}
routes={routes}
routeNamePath={standardConfig.route_name_path}
onChange={onChange("route_name_path")}
errors={get(errors, "route_name_path")}
/>
</EuiFlexItem>
) : (
<EuiFlexItem>
<FallbackView text={"Loading ..."} />
</EuiFlexItem>
)}

<EuiFlexItem>
<RouteSelectionPanel
routeId={standardConfig.fallback_response_route_id}
Expand Down
Loading