parshimers commented on code in PR #6897:
URL: https://github.com/apache/texera/pull/6897#discussion_r4009942743


##########
amber/src/main/scala/org/apache/texera/web/service/ExecutionReconfigurationService.scala:
##########
@@ -90,7 +91,10 @@ class ExecutionReconfigurationService(
 
     val reconfigurationId = UUID.randomUUID().toString
     val updateExecutorRequests = reconfigurations.map {
-      case (op, _) => UpdateExecutorRequest(op.id, op.opExecInitInfo)
+      case (op, _) =>
+        // An edited UDF may now name a repository, and its code already 
refers to the mount.
+        ensureMounted(op.mountLocators)

Review Comment:
   what happens here if op.mountLocators is empty or null?



##########
amber/src/main/scala/org/apache/texera/amber/engine/architecture/scheduling/RegionExecutionManager.scala:
##########
@@ -352,6 +357,23 @@ class RegionExecutionManager(
     )
   }
 
+  /**
+    * Mount every repository this region's operators name before any of them 
receives its code,
+    * which already refers to the mount paths. It happens here rather than in 
a worker because
+    * workers are created before either phase launches. Operators carry no 
mount code: they only
+    * name what they need, through `PhysicalOp.mountLocators`.
+    */
+  private def mountRegionRepositories(): Future[Unit] = {
+    val mount = Promise[Unit]()
+    if (mountFutureRef.compareAndSet(null, mount)) {
+      mount.become(Future {
+        val locators = region.getOperators.flatMap(_.mountLocators)
+        if (locators.nonEmpty) ensureMounted(locators)

Review Comment:
   claude seems to think this blocks this thread, and that it will be the 
Coordinator thread, so the operator can't be paused while the mounts are 
processed. is it the case?



##########
frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts:
##########
@@ -104,8 +111,17 @@ export class UiUdfParametersComponent extends 
FieldArrayType<FormlyFieldConfig>
     this.configureRowColumns(rowField, this.setDisabledMetadata.bind(this));
   }
 
-  private configureRowFields(rowField: FormlyFieldConfig | undefined): void {
+  private configureRowFields(rowField: FormlyFieldConfig | undefined, 
inputType?: string): void {
     this.configureRowColumns(rowField, this.configureDisabledState.bind(this));
+    this.configureValueEditor(rowField, inputType);
+  }
+
+  /** A row whose value names a resource is edited with that resource's 
browser, not a text box. */
+  private configureValueEditor(rowField: FormlyFieldConfig | undefined, 
inputType?: string): void {
+    const valueField = rowField && this.getColumnField(rowField, VALUE_COLUMN);
+    if (!valueField || !inputType || !RESOURCE_INPUT_TYPES.has(inputType)) 
return;
+    valueField.type = RESOURCE_VALUE_EDITOR;

Review Comment:
   should this ever be set to null or some other field, once the above guard is 
passed?



-- 
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]

Reply via email to