kunwp1 commented on code in PR #5069:
URL: https://github.com/apache/texera/pull/5069#discussion_r3269884814
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/source/PythonUDFSourceOpDescV2.scala:
##########
@@ -49,6 +49,18 @@ class PythonUDFSourceOpDescV2 extends
SourceOperatorDescriptor {
@JsonPropertyDescription("Specify how many parallel workers to launch")
var workers: Int = 1
+ @JsonProperty(required = true, defaultValue = "true")
+ @JsonSchemaTitle("Default Python Environment")
+ @JsonPropertyDescription("Use Default Python Environment\"")
Review Comment:
Is adding `\"` a typo? Happening to all the operator descriptors.
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -173,9 +177,29 @@ export class OperatorPropertyEditFrameComponent implements
OnInit, OnChanges, On
private changeDetectorRef: ChangeDetectorRef,
private workflowVersionService: WorkflowVersionService,
private workflowStatusSerivce: WorkflowStatusService,
- private config: GuiConfigService
+ private config: GuiConfigService,
+ private workflowPveService: WorkflowPveService,
+ private computingUnitStatusService: ComputingUnitStatusService
) {}
+ private patchPythonUdfEnvironmentSchema(schema: CustomJSONSchema7,
environments: string[]): CustomJSONSchema7 {
+ const patchedSchema = cloneDeep(schema);
+
+ if (patchedSchema.properties && typeof patchedSchema.properties !==
"boolean") {
+ const envProperty = patchedSchema.properties["envName"] as
CustomJSONSchema7;
+ envProperty.enum = environments;
+ }
+
+ return patchedSchema;
+ }
+
+ private hideEnvNameWhenDefaultEnvChecked(): void {
+ const envField = this.formlyFields?.[0]?.fieldGroup?.find(f => f.key ===
"envName");
+ if (envField) {
+ envField.expressions = { hide: "!!field.parent.model.defaultEnv" };
Review Comment:
Add `required: true` on `envName` when `defaultEnv` is `false`
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUDFOpDescV2.scala:
##########
@@ -140,6 +152,7 @@ class PythonUDFOpDescV2 extends LogicalOp {
.withPartitionRequirement(partitionRequirement)
.withIsOneToManyOp(true)
.withPropagateSchema(SchemaPropagationFunc(propagateSchema))
+ .withPveName(if (defaultEnv) "" else envName.trim)
Review Comment:
ditto
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/DualInputPortsPythonUDFOpDescV2.scala:
##########
@@ -129,6 +141,7 @@ class DualInputPortsPythonUDFOpDescV2 extends LogicalOp {
Map(operatorInfo.outputPorts.head.id -> outputSchema)
})
)
+ .withPveName(if (defaultEnv) "" else envName.trim)
Review Comment:
ditto
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/source/PythonUDFSourceOpDescV2.scala:
##########
@@ -68,6 +80,7 @@ class PythonUDFSourceOpDescV2 extends
SourceOperatorDescriptor {
SchemaPropagationFunc(_ => Map(operatorInfo.outputPorts.head.id ->
sourceSchema()))
)
.withLocationPreference(Option.empty)
+ .withPveName(if (defaultEnv) "" else envName.trim)
Review Comment:
If a user unchecks "Default Python Environment" but leaves envName empty,
the form silently submits and `withPveName(if (defaultEnv) "" else
envName.trim)` resolves to `""` which falls back to default Python. Can you add
a check to handle the case where `defaultEnv: false, envName: ""`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
