Skip to content

Commit 278f190

Browse files
committed
Updates from feedback
1 parent fc5e9fc commit 278f190

File tree

6 files changed

+21
-24
lines changed

6 files changed

+21
-24
lines changed

packages/model-serving/src/components/deploymentWizard/ModelDeploymentWizard.tsx

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,21 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
5353
// External data state - loaded by ExternalDataLoader component
5454
const [externalData, setExternalData] = React.useState<ExternalDataMap>({});
5555

56-
const wizardState = useModelDeploymentWizard(existingData, project?.metadata.name, externalData);
57-
const validation = useModelDeploymentWizardValidation(wizardState.state, wizardState.fields);
58-
const currentProjectName = wizardState.state.project.projectName ?? undefined;
56+
const currentProjectName = project?.metadata.name;
5957

6058
const [canCreateRoleBindings] = useAccessReview({
6159
...accessReviewResource,
6260
namespace: currentProjectName,
6361
});
6462

63+
const wizardState = useModelDeploymentWizard(
64+
existingData,
65+
currentProjectName,
66+
externalData,
67+
canCreateRoleBindings,
68+
);
69+
const validation = useModelDeploymentWizardValidation(wizardState.state, wizardState.fields);
70+
6571
const { deployMethod, deployMethodLoaded } = useDeployMethod(wizardState.state);
6672
// TODO in same jira, replace deployMethod with applyFieldData for all other fields
6773
const { applyFieldData, applyExtensionsLoaded } = useWizardFieldApply(wizardState.state);
@@ -145,10 +151,7 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
145151
serverResource,
146152
serverResourceTemplateName,
147153
overwrite,
148-
{
149-
...wizardState.initialData,
150-
canCreateRoleBindings,
151-
},
154+
wizardState.initialData,
152155
applyFieldData,
153156
);
154157
} catch (error) {
@@ -160,7 +163,6 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
160163
[
161164
applyExtensionsLoaded,
162165
applyFieldData,
163-
canCreateRoleBindings,
164166
deployMethod,
165167
deployMethodLoaded,
166168
existingDeployment,
@@ -253,8 +255,8 @@ const ModelDeploymentWizard: React.FC<ModelDeploymentWizardProps> = ({
253255
{wizardState.loaded.advancedOptionsLoaded ? (
254256
<AdvancedSettingsStepContent
255257
wizardState={wizardState}
256-
projectName={currentProjectName}
257258
externalData={externalData}
259+
allowCreate={canCreateRoleBindings}
258260
/>
259261
) : (
260262
<Spinner data-testid="spinner" />

packages/model-serving/src/components/deploymentWizard/fields/EnvironmentVariablesField.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ type EnvironmentVariablesFieldProps = {
9595
export const EnvironmentVariablesField: React.FC<EnvironmentVariablesFieldProps> = ({
9696
data = { enabled: false, variables: [] },
9797
onChange,
98-
allowCreate = false,
98+
allowCreate = true,
9999
predefinedVars,
100100
}) => {
101101
const lastNameFieldRef = React.useRef<HTMLInputElement>(null);

packages/model-serving/src/components/deploymentWizard/fields/RuntimeArgsField.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ type RuntimeArgsFieldProps = {
5555
export const RuntimeArgsField: React.FC<RuntimeArgsFieldProps> = ({
5656
data = { enabled: false, args: [] },
5757
onChange,
58-
allowCreate = false,
58+
allowCreate = true,
5959
predefinedArgs,
6060
}) => {
6161
const handleCheckboxChange = (event: React.FormEvent<HTMLInputElement>, checked: boolean) => {

packages/model-serving/src/components/deploymentWizard/steps/AdvancedOptionsStep.tsx

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
} from '@odh-dashboard/internal/k8sTypes';
88
import { isServingRuntimeKind } from '@odh-dashboard/internal/pages/modelServing/customServingRuntimes/utils';
99
import { Form, Stack, StackItem, Alert, FormGroup, FormSection } from '@patternfly/react-core';
10-
import { useAccessReview } from '@odh-dashboard/internal/api/index';
1110
import { ExternalRouteField } from '../fields/ExternalRouteField';
1211
import { TokenAuthenticationField } from '../fields/TokenAuthenticationField';
1312
import { RuntimeArgsField } from '../fields/RuntimeArgsField';
@@ -26,14 +25,14 @@ export const accessReviewResource: AccessReviewResourceAttributes = {
2625

2726
type AdvancedSettingsStepContentProps = {
2827
wizardState: UseModelDeploymentWizardState;
29-
projectName?: string;
3028
externalData: ExternalDataMap;
29+
allowCreate: boolean;
3130
};
3231

3332
export const AdvancedSettingsStepContent: React.FC<AdvancedSettingsStepContentProps> = ({
3433
wizardState,
35-
projectName,
3634
externalData,
35+
allowCreate,
3736
}) => {
3837
const externalRouteData = wizardState.state.externalRoute.data;
3938
const tokenAuthData = wizardState.state.tokenAuthentication.data;
@@ -84,11 +83,6 @@ export const AdvancedSettingsStepContent: React.FC<AdvancedSettingsStepContentPr
8483
return kserveContainer.env?.map((ev) => `${ev.name}=${ev.value ?? ''}`) || [];
8584
};
8685

87-
const [allowCreate] = useAccessReview({
88-
...accessReviewResource,
89-
namespace: projectName,
90-
});
91-
9286
const handleExternalRouteChange = (checked: boolean) => {
9387
wizardState.state.externalRoute.setData(checked);
9488

@@ -181,15 +175,13 @@ export const AdvancedSettingsStepContent: React.FC<AdvancedSettingsStepContentPr
181175
<RuntimeArgsField
182176
data={wizardState.state.runtimeArgs.data}
183177
onChange={wizardState.state.runtimeArgs.setData}
184-
allowCreate={allowCreate}
185178
predefinedArgs={getKServeContainerArgs(selectedModelServer)}
186179
/>
187180
</StackItem>
188181
<StackItem>
189182
<EnvironmentVariablesField
190183
data={wizardState.state.environmentVariables.data}
191184
onChange={wizardState.state.environmentVariables.setData}
192-
allowCreate={allowCreate}
193185
predefinedVars={getKServeContainerEnvVarStrs(selectedModelServer)}
194186
/>
195187
</StackItem>

packages/model-serving/src/components/deploymentWizard/useDeploymentWizard.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import { useDeploymentWizardReducer, type WizardFormAction } from './useDeployme
2323
import type { ExternalDataMap } from './ExternalDataLoader';
2424

2525
export type UseModelDeploymentWizardState = WizardFormData & {
26+
canCreateRoleBindings: boolean;
2627
loaded: {
2728
modelSourceLoaded: boolean;
2829
modelDeploymentLoaded: boolean;
@@ -42,6 +43,7 @@ export const useModelDeploymentWizard = (
4243
initialData?: InitialWizardFormData,
4344
initialProjectName?: string | undefined,
4445
externalDataMap: ExternalDataMap = {},
46+
canCreateRoleBindings = true,
4547
): UseModelDeploymentWizardState => {
4648
// Step 1: Model Source
4749
const modelType = useModelTypeField(initialData?.modelTypeField);
@@ -171,6 +173,7 @@ export const useModelDeploymentWizard = (
171173
state,
172174
dispatch,
173175
fields,
176+
canCreateRoleBindings,
174177
loaded: {
175178
modelSourceLoaded,
176179
modelDeploymentLoaded,

packages/model-serving/src/components/deploymentWizard/utils.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export const getTokenAuthenticationFromDeployment = (
7979
};
8080

8181
export const deployModel = async (
82-
wizardState: WizardFormData,
82+
wizardState: WizardFormData & { canCreateRoleBindings: boolean },
8383
secretName: string,
8484
exitWizard: () => void,
8585
deployMethod?: (
@@ -126,7 +126,7 @@ export const deployModel = async (
126126
true,
127127
undefined,
128128
undefined,
129-
initialWizardData,
129+
{ ...initialWizardData, canCreateRoleBindings: wizardState.canCreateRoleBindings },
130130
applyFieldData,
131131
),
132132
]
@@ -160,7 +160,7 @@ export const deployModel = async (
160160
false,
161161
actualSecretName,
162162
overwrite,
163-
initialWizardData,
163+
{ ...initialWizardData, canCreateRoleBindings: wizardState.canCreateRoleBindings },
164164
applyFieldData,
165165
);
166166

0 commit comments

Comments
 (0)