You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "carloea2 (via GitHub)" <gi...@apache.org> on 2026/03/07 09:12:43 UTC
[PR] Feat/UI parameter [texera]
carloea2 opened a new pull request, #4268:
URL: https://github.com/apache/texera/pull/4268
<!--
Thanks for sending a pull request (PR)! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
[Contributing to Texera](https://github.com/apache/texera/blob/main/CONTRIBUTING.md)
2. Ensure you have added or run the appropriate tests for your PR
3. If the PR is work in progress, mark it a draft on GitHub.
4. Please write your PR title to summarize what this PR proposes, we
are following Conventional Commits style for PR titles as well.
5. Be sure to keep the PR description updated to reflect all changes.
-->
### What changes were proposed in this PR?
Add Python UDF Parameter support:
<img width="1594" height="601" alt="image" src="https://github.com/user-attachments/assets/771f2c85-1442-4c9a-98cf-f5c41e8d2707" />
### What the user writes (Python)
Users declare UI parameters once in `open()`, and then use the typed value directly:
```python
from pytexera import *
from typing import Iterator, Optional
class ProcessTupleOperator(UDFOperatorV2):
@overrides
def open(self):
# declare UiParameters once, store the parsed runtime values
self.value1 = self.UiParameter(name="param1", type=AttributeType.DOUBLE).value
self.value2 = self.UiParameter(name="param2", type=AttributeType.INT).value
self.value3 = self.UiParameter(name="param3", type=AttributeType.STRING).value
self.value4 = self.UiParameter(name="param4", type=AttributeType.TIMESTAMP).value
@overrides
def process_tuple(self, tuple_: Tuple, port: int) -> Iterator[Optional[TupleLike]]:
print(self.value1)
print(self.value2)
print(self.value3)
print(self.value4)
yield tuple_
```
### What shows up in the UI
From those `self.UiParameter(...)` lines, the property panel automatically generates a **Parameters** section with one row per parameter:
* **Name** + **Type** are read-only (so they stay consistent with the code)
* **Value** is editable (so users can change runtime values without touching the script)
### How the values get into Python
When the workflow runs, we inject the UI values into the UDF and the base class applies them right before `open()` executes. That way, when the user calls `UiParameter(...).value`, they get the current value from the UI.
### Any related issues, documentation, discussions?
Closes [#4154
](https://github.com/apache/texera/discussions/4154)
### How was this PR tested?
Testing added to backend and frontend
### Was this PR authored or co-authored using generative AI tooling?
Co-generated with GPT
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4248775489
@kunwp1
Thanks.
1. Ok.
2. I will remove them.
3. Got it.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4309041057
We should aim to keep PRs as small as possible unless you can clearly prove that a PR cannot be split. I don’t think that’s the case here.
Breaking a larger prototype into smaller PRs can also help you think more carefully about how to modularize your code.
While we don’t have a strict limit on PR size, in my experience, anything with more than ~500 lines of additions is usually too large—unless it’s primarily refactoring (e.g., moving existing code without significant changes) or includes generated files.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4310537192
It's all about communication: in Pr context, it is communication between PR author and the reviewer. Larger PRs usually get fewer and shallower reviews. A huge PR is almost equivalent to being unreviewable.
I agree with Xinyuan. A reasonable rule of thumb is that the source-code of a PR should ideally be around 200 LoC, with 500 LoC as a rough upper bound, and should usually touch fewer than 10 files. Test files can be a bit bigger and usually reviewers pay less effort to review tests.
Of course, this also depends on the language and the nature of the change.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025313407
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts:
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Injectable } from "@angular/core";
+import { isEqual } from "lodash-es";
+import { ReplaySubject } from "rxjs";
+import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service";
+import { UiUdfParameter, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service";
+import { isDefined } from "../../../common/util/predicate";
+import {
+ DUAL_INPUT_PORTS_PYTHON_UDF_V2_OP_TYPE,
+ PYTHON_UDF_SOURCE_V2_OP_TYPE,
+ PYTHON_UDF_V2_OP_TYPE,
+} from "../workflow-graph/model/workflow-graph";
+import { YType } from "../../types/shared-editing.interface";
+import { YText } from "yjs/dist/src/types/YText";
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersSyncService {
+ private readonly uiParametersChangedSubject = new ReplaySubject<{ operatorId: string; parameters: UiUdfParameter[] }>(
+ 1
+ );
+
+ readonly uiParametersChanged$ = this.uiParametersChangedSubject.asObservable();
+
+ constructor(
+ private workflowActionService: WorkflowActionService,
+ private uiUdfParametersParserService: UiUdfParametersParserService
+ ) {}
+
+ /**
+ * Attach directly to YText and sync whenever it changes
+ */
+ attachToYCode(operatorId: string, yCode: YText): () => void {
+ const handler = () => {
+ const latestCode = yCode.toString();
+ this.syncStructureFromCode(operatorId, latestCode);
+ };
+
+ yCode.observe(handler);
+
+ handler();
+
+ // return cleanup function
+ return () => yCode.unobserve(handler);
+ }
+
+ syncStructureFromCode(operatorId: string, codeFromEditor?: string): void {
+ const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId);
+
+ if (!operator || !this.isSupportedPythonUdfType(operator.operatorType)) {
+ return;
+ }
+
+ const code = codeFromEditor ?? this.getSharedCode(operatorId);
+ if (!isDefined(code)) {
+ return;
+ }
+
+ const existingParameters = operator.operatorProperties?.uiParameters ?? [];
+ const mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters);
+
+ if (isEqual(existingParameters, mergedUiParameters)) {
+ return;
+ }
+
+ // Emit event so UI updates
+ this.uiParametersChangedSubject.next({
+ operatorId,
+ parameters: mergedUiParameters,
+ });
+
+ // optionally persist here if desired
+ // this.workflowActionService.setOperatorProperty(...)
+ }
+
+ private buildParsedShapeWithPreservedValues(code: string, existingParameters: any[]): UiUdfParameter[] {
+ const parsedParameters = this.uiUdfParametersParserService.parse(code);
+
+ const existingValues = new Map<string, string>();
+ existingParameters.forEach((parameter: any) => {
+ const parameterName = parameter?.attribute?.attributeName ?? parameter?.attribute?.name;
+
+ if (isDefined(parameterName) && isDefined(parameter?.value)) {
+ existingValues.set(parameterName, parameter.value);
+ }
+ });
+
+ return parsedParameters.map(parameter => ({
+ ...parameter,
+ value: existingValues.get(parameter.attribute.attributeName) ?? "",
+ }));
+ }
Review Comment:
Good
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025308343
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -193,6 +194,23 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
this.currentOperatorStatus = update[this.currentOperatorId];
}
});
+
+ this.uiUdfParametersSyncService.uiParametersChanged$
+ .pipe(untilDestroyed(this))
+ .subscribe(({ operatorId, parameters }) => {
+ if (operatorId !== this.currentOperatorId) return;
+
+ const currentOperator = this.workflowActionService.getTexeraGraph().getOperator(operatorId);
+
+ const newModel = {
+ ...cloneDeep(currentOperator.operatorProperties),
+ uiParameters: cloneDeep(parameters),
+ };
+
+ this.listeningToChange = false;
+ this.workflowActionService.setOperatorProperty(operatorId, newModel);
+ this.listeningToChange = true;
Review Comment:
No, this is the intended way to avoid infinite loops.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r2938436781
##########
frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html:
##########
@@ -0,0 +1,55 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+<div
+ class="ui-udf-param-list"
+ *ngIf="(model?.length ?? 0) > 0">
+ <!-- Optional header row -->
+ <div class="ui-udf-param-row header">
+ <div class="field-cell"><span class="col-title">Value</span></div>
+ <div class="field-cell"><span class="col-title">Name</span></div>
+ <div class="field-cell"><span class="col-title">Type</span></div>
+ </div>
+
+ <div
+ class="ui-udf-param-row"
+ *ngFor="let param of (model || []); let i = index; trackBy: trackByParamName">
+ <ng-container *ngIf="field.fieldGroup?.[i] as rowField">
+ <!-- Value -->
+ <div class="field-cell">
+ <ng-container *ngIf="getValueField(rowField) as valueField">
+ <formly-field [field]="valueField"></formly-field>
+ </ng-container>
+ </div>
+
+ <!-- Name -->
+ <div class="field-cell">
+ <ng-container *ngIf="getNameField(rowField) as nameField">
+ <formly-field [field]="nameField"></formly-field>
+ </ng-container>
+ </div>
+
+ <!-- Type -->
+ <div class="field-cell">
+ <ng-container *ngIf="getTypeField(rowField) as typeField">
+ <formly-field [field]="typeField"></formly-field>
+ </ng-container>
+ </div>
+ </ng-container>
+ </div>
+</div>
Review Comment:
I think copilot was too high in vibe coding, the Screenshot is "Value", "Name", "Type", and the HTML is also "Value", "Name", "Type". I do not even think copilot actually use any screenshot at all.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104563235
##########
amber/src/main/python/pyamber/__init__.py:
##########
Review Comment:
No, this change is needed for the feature to work. The reason is to load the definition of AttributeType when importing.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104689100
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/wordCloud/WordCloudOpDesc.scala:
##########
Review Comment:
There is an issue that our CI check doesn't enforce scalafixAll. I will raise a PR to fix it. Those changes should disappear after I merge the PR.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4302160922
The PR is too big that reviewers couldn't finish the review.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4505486084
@carloea2 Sorry for this very late reply. As you know, over the past few days we have been very busy with the dkNet-AI · Apache Texera Agent Hackathon event (https://texera.io/hackathon/) and post-event tasks.
Thanks a lot for sharing your feedback, which is very useful to help us manage this project and grow the community. I notice your post mentioned several topics. To have efficient communication, I suggest you create multiple discussion posts on GitHub and add the URLs in this issue (similar to our practice of dividing a big PR into smaller PRs). Then we can continue the discussion in those posts.
Thank you.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4524340936
@chenlica
After communicating with other team members, I can say that my concerns have been addressed. I appreciate your time and the team’s efforts despite the busy schedule.
Thank you.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r2938455794
##########
frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html:
##########
@@ -0,0 +1,55 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+<div
+ class="ui-udf-param-list"
+ *ngIf="(model?.length ?? 0) > 0">
+ <!-- Optional header row -->
+ <div class="ui-udf-param-row header">
+ <div class="field-cell"><span class="col-title">Value</span></div>
+ <div class="field-cell"><span class="col-title">Name</span></div>
+ <div class="field-cell"><span class="col-title">Type</span></div>
+ </div>
+
+ <div
+ class="ui-udf-param-row"
+ *ngFor="let param of (model || []); let i = index; trackBy: trackByParamName">
+ <ng-container *ngIf="field.fieldGroup?.[i] as rowField">
+ <!-- Value -->
+ <div class="field-cell">
+ <ng-container *ngIf="getValueField(rowField) as valueField">
+ <formly-field [field]="valueField"></formly-field>
+ </ng-container>
+ </div>
+
+ <!-- Name -->
+ <div class="field-cell">
+ <ng-container *ngIf="getNameField(rowField) as nameField">
+ <formly-field [field]="nameField"></formly-field>
+ </ng-container>
+ </div>
+
+ <!-- Type -->
+ <div class="field-cell">
+ <ng-container *ngIf="getTypeField(rowField) as typeField">
+ <formly-field [field]="typeField"></formly-field>
+ </ng-container>
+ </div>
+ </ng-container>
+ </div>
+</div>
Review Comment:
Yeah. AIs are usually not very good at reading text in images. We’re just using AI as a helper—we never treat its responses as the truth. If a comment is incorrect, simply reply to it and resolve the comment.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3035948986
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
+ "STRING",
+ "INTEGER",
+ "LONG",
+ "DOUBLE",
+ "BOOLEAN",
+ "TIMESTAMP",
+ "BINARY",
+ "LARGE_BINARY",
+] as const;
+
+export type JavaAttributeTypeName = (typeof JAVA_ATTRIBUTE_TYPE_NAMES)[number];
+
+// Python enum constant names (core.models.AttributeType)
+export const PYTHON_ATTRIBUTE_TYPE_NAMES = [
+ "STRING",
+ "INT",
+ "LONG",
+ "DOUBLE",
+ "BOOL",
+ "TIMESTAMP",
+ "BINARY",
+ "LARGE_BINARY",
+] as const;
+
+export type PythonAttributeTypeName = (typeof PYTHON_ATTRIBUTE_TYPE_NAMES)[number];
+
+// Useful when parsing code from either side
+export type AttributeTypeToken = AttributeType | JavaAttributeTypeName | PythonAttributeTypeName;
Review Comment:
It is because Python names and Java names for attributes are not the same for all, such as Int and Integer
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104746385
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/wordCloud/WordCloudOpDesc.scala:
##########
Review Comment:
PR #4411
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4249805312
@carloea2 Please resolve conflicts.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105590272
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
1) Ok.
2) Ok.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025288848
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,17 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: int,
+ AttributeType.LONG: int,
+ AttributeType.DOUBLE: float,
+ AttributeType.BOOL: lambda v: str(v).strip().lower() in ("true", "1", "yes"),
+ AttributeType.BINARY: lambda v: v if isinstance(v, bytes) else str(v).encode(),
+ AttributeType.TIMESTAMP: lambda v: datetime.datetime.fromisoformat(v),
+ AttributeType.LARGE_BINARY: largebinary,
+}
Review Comment:
`AttributeType.TIMESTAMP` parsing uses `datetime.datetime.fromisoformat(v)`, which raises `ValueError` for common ISO-8601 inputs with a trailing `Z` (e.g., `2024-01-01T00:00:00Z`, which is produced by many Java/Scala serializers). This will break UiParameter timestamp values coming from the UI/backend. Update the parser to handle `Z`/offset forms (e.g., normalize `Z` to `+00:00`) and add a test case for the `Z` form.
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
+ }
+
+ entries.reduceOption((acc, entry) => acc + pyb", " + entry).getOrElse(pyb"")
+ }
+
+ private def buildInjectedHookMethod(uiParameters: List[UiUDFParameter]): String = {
+ val injectedParametersMap = buildInjectedParametersMap(uiParameters)
+
+ // unindented method; we indent it when inserting into the class body
+ (pyb"""|@overrides
+ |def """ + pyb"$ReservedHookMethod" + pyb"""(self) -> typing.Dict[str, typing.Any]:
+ | return {""" +
+ injectedParametersMap +
+ pyb"""}
+ |""").encode
+ }
+
+ private def ensureTypingImport(encodedUserCode: String): String = {
+ val alreadyImported = encodedUserCode
+ .split("\n", -1)
+ .exists(_.trim == "import typing")
+
+ if (alreadyImported) {
+ encodedUserCode
+ } else {
+ "import typing\n" + encodedUserCode
+ }
Review Comment:
`ensureTypingImport` prepends `import typing` to the very top of the user script. This can break valid Python that requires header ordering (notably `from __future__ import ...` must appear before any other imports, and a shebang/encoding cookie may need to remain first). Consider inserting `import typing` after any shebang/encoding lines and any `__future__` imports instead of always prefixing.
```suggestion
val lines = encodedUserCode.split("\n", -1)
// If 'import typing' is already present as a standalone line, leave the code unchanged.
val alreadyImported = lines.exists(_.trim == "import typing")
if (alreadyImported) {
return encodedUserCode
}
// Determine where to insert 'import typing' while respecting Python header ordering:
// - shebang line (#!...)
// - encoding cookie (e.g., # -*- coding: utf-8 -*- or # coding=utf-8)
// - 'from __future__ import ...' imports
var insertAt = 0
var i = 0
// Skip shebang line if present.
if (lines.nonEmpty && lines(0).startsWith("#!")) {
insertAt = 1
i = 1
}
// Regex for encoding declaration comments, case-insensitive on "coding".
val encodingRegex: Regex = """(?i)^#.*coding[:=]\s*[-\w.]+""".r
// Advance past encoding cookies and __future__ imports.
while (i < lines.length) {
val trimmed = lines(i).trim
trimmed match {
case encodingRegex() =>
insertAt = i + 1
i += 1
case t if t.startsWith("from __future__ import ") =>
insertAt = i + 1
i += 1
case _ =>
// Stop at the first line that is not an encoding cookie or __future__ import.
i = lines.length
}
}
val newLines =
if (insertAt >= lines.length) {
// Append at the end (unlikely, but safe).
lines.toList :+ "import typing"
} else {
val (before, after) = lines.splitAt(insertAt)
before.toList ::: "import typing" :: after.toList
}
newLines.mkString("\n")
```
##########
frontend/src/app/workspace/component/property-editor/operator-property-edit-frame/operator-property-edit-frame.component.ts:
##########
@@ -193,6 +194,23 @@ export class OperatorPropertyEditFrameComponent implements OnInit, OnChanges, On
this.currentOperatorStatus = update[this.currentOperatorId];
}
});
+
+ this.uiUdfParametersSyncService.uiParametersChanged$
+ .pipe(untilDestroyed(this))
+ .subscribe(({ operatorId, parameters }) => {
+ if (operatorId !== this.currentOperatorId) return;
+
+ const currentOperator = this.workflowActionService.getTexeraGraph().getOperator(operatorId);
+
+ const newModel = {
+ ...cloneDeep(currentOperator.operatorProperties),
+ uiParameters: cloneDeep(parameters),
+ };
+
+ this.listeningToChange = false;
+ this.workflowActionService.setOperatorProperty(operatorId, newModel);
+ this.listeningToChange = true;
Review Comment:
In the `uiParametersChanged$` subscription, `listeningToChange` is set to `false` around `setOperatorProperty`. Since operator property updates are emitted synchronously via Yjs transactions, this prevents `registerOperatorPropertyChangeHandler()` from updating `formData`, so the property panel may not reflect the newly inferred `uiParameters`. Consider either not disabling `listeningToChange` here, or explicitly updating `this.formData`/rerendering after `setOperatorProperty`.
```suggestion
this.workflowActionService.setOperatorProperty(operatorId, newModel);
```
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts:
##########
@@ -0,0 +1,130 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Injectable } from "@angular/core";
+import { isEqual } from "lodash-es";
+import { ReplaySubject } from "rxjs";
+import { WorkflowActionService } from "../workflow-graph/model/workflow-action.service";
+import { UiUdfParameter, UiUdfParametersParserService } from "./ui-udf-parameters-parser.service";
+import { isDefined } from "../../../common/util/predicate";
+import {
+ DUAL_INPUT_PORTS_PYTHON_UDF_V2_OP_TYPE,
+ PYTHON_UDF_SOURCE_V2_OP_TYPE,
+ PYTHON_UDF_V2_OP_TYPE,
+} from "../workflow-graph/model/workflow-graph";
+import { YType } from "../../types/shared-editing.interface";
+import { YText } from "yjs/dist/src/types/YText";
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersSyncService {
+ private readonly uiParametersChangedSubject = new ReplaySubject<{ operatorId: string; parameters: UiUdfParameter[] }>(
+ 1
+ );
+
+ readonly uiParametersChanged$ = this.uiParametersChangedSubject.asObservable();
+
+ constructor(
+ private workflowActionService: WorkflowActionService,
+ private uiUdfParametersParserService: UiUdfParametersParserService
+ ) {}
+
+ /**
+ * Attach directly to YText and sync whenever it changes
+ */
+ attachToYCode(operatorId: string, yCode: YText): () => void {
+ const handler = () => {
+ const latestCode = yCode.toString();
+ this.syncStructureFromCode(operatorId, latestCode);
+ };
+
+ yCode.observe(handler);
+
+ handler();
+
+ // return cleanup function
+ return () => yCode.unobserve(handler);
+ }
+
+ syncStructureFromCode(operatorId: string, codeFromEditor?: string): void {
+ const operator = this.workflowActionService.getTexeraGraph().getOperator(operatorId);
+
+ if (!operator || !this.isSupportedPythonUdfType(operator.operatorType)) {
+ return;
+ }
+
+ const code = codeFromEditor ?? this.getSharedCode(operatorId);
+ if (!isDefined(code)) {
+ return;
+ }
+
+ const existingParameters = operator.operatorProperties?.uiParameters ?? [];
+ const mergedUiParameters = this.buildParsedShapeWithPreservedValues(code, existingParameters);
+
+ if (isEqual(existingParameters, mergedUiParameters)) {
+ return;
+ }
+
+ // Emit event so UI updates
+ this.uiParametersChangedSubject.next({
+ operatorId,
+ parameters: mergedUiParameters,
+ });
+
+ // optionally persist here if desired
+ // this.workflowActionService.setOperatorProperty(...)
+ }
+
+ private buildParsedShapeWithPreservedValues(code: string, existingParameters: any[]): UiUdfParameter[] {
+ const parsedParameters = this.uiUdfParametersParserService.parse(code);
+
+ const existingValues = new Map<string, string>();
+ existingParameters.forEach((parameter: any) => {
+ const parameterName = parameter?.attribute?.attributeName ?? parameter?.attribute?.name;
+
+ if (isDefined(parameterName) && isDefined(parameter?.value)) {
+ existingValues.set(parameterName, parameter.value);
+ }
+ });
+
+ return parsedParameters.map(parameter => ({
+ ...parameter,
+ value: existingValues.get(parameter.attribute.attributeName) ?? "",
+ }));
+ }
Review Comment:
This new sync service drives core UX (auto-infer parameter shape from code and preserve values), but it currently has no unit tests. Since this directory already uses Angular/Jasmine tests (e.g., `ui-udf-parameters-parser.service.spec.ts`), consider adding tests for `syncStructureFromCode` / `buildParsedShapeWithPreservedValues` to cover preserving values, dropping removed params, and emitting only when the merged model changes.
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
+ }
+
+ entries.reduceOption((acc, entry) => acc + pyb", " + entry).getOrElse(pyb"")
+ }
+
+ private def buildInjectedHookMethod(uiParameters: List[UiUDFParameter]): String = {
+ val injectedParametersMap = buildInjectedParametersMap(uiParameters)
+
+ // unindented method; we indent it when inserting into the class body
+ (pyb"""|@overrides
+ |def """ + pyb"$ReservedHookMethod" + pyb"""(self) -> typing.Dict[str, typing.Any]:
+ | return {""" +
+ injectedParametersMap +
+ pyb"""}
+ |""").encode
Review Comment:
The injected hook method is decorated with `@overrides`, but the injector only ensures `import typing` is present. If a user script does not import `overrides` (e.g., uses `import pytexera` instead of `from pytexera import *`), the injected code will fail at import time with `NameError: name 'overrides' is not defined`. Consider removing the decorator from the injected method or injecting an explicit `from overrides import overrides` / `import overrides` consistently.
```suggestion
(pyb"""|def """ + pyb"$ReservedHookMethod" + pyb"""(self) -> typing.Dict[str, typing.Any]:
| return {""" +
injectedParametersMap +
pyb"""}
|""").encode
```
##########
frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.ts:
##########
@@ -0,0 +1,91 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Component } from "@angular/core";
+import { FieldArrayType, FormlyFieldConfig } from "@ngx-formly/core";
+
+@Component({
+ selector: "texera-ui-udf-parameters",
+ templateUrl: "./ui-udf-parameters.component.html",
+ styleUrls: ["./ui-udf-parameters.component.scss"],
+})
+export class UiUdfParametersComponent extends FieldArrayType {
+ private getField(rowField: FormlyFieldConfig, key: string): FormlyFieldConfig | undefined {
+ return rowField.fieldGroup?.find(f => f.key === key);
+ }
+
+ private getAttributeChild(rowField: FormlyFieldConfig, childKey: string): FormlyFieldConfig | undefined {
+ const attributeGroup = this.getField(rowField, "attribute");
+ return attributeGroup?.fieldGroup?.find(f => f.key === childKey);
+ }
+
+ private setDisabled(field: FormlyFieldConfig | undefined, disabled: boolean): FormlyFieldConfig | undefined {
+ if (!field) return undefined;
+
+ // 1) Modern Formly
+ field.props = { ...(field.props ?? {}), disabled };
+
+ // 2) Compatibility for templates/wrappers still using templateOptions
+ // (use `as any` so you don't get nagged by the @deprecated JSDoc)
+ (field as any).templateOptions = { ...((field as any).templateOptions ?? {}), disabled };
+
+ // 3) Enforce at the reactive form level
+ if (field.formControl) {
+ if (disabled) {
+ field.formControl.disable({ emitEvent: false });
+ } else {
+ field.formControl.enable({ emitEvent: false });
+ }
+ } else {
+ // If control isn't created yet, disable it at init time.
+ const prevOnInit = field.hooks?.onInit;
+ field.hooks = {
+ ...(field.hooks ?? {}),
+ onInit: f => {
+ prevOnInit?.(f);
+ if (disabled) {
+ f.formControl?.disable({ emitEvent: false });
+ } else {
+ f.formControl?.enable({ emitEvent: false });
+ }
+ },
+ };
Review Comment:
`setDisabled()` mutates `field.hooks.onInit` by wrapping it every time the template calls `getNameField/getTypeField/getValueField`. Since these getters run during change detection, this can repeatedly nest `onInit` wrappers (and reapply disable/enable logic multiple times). Consider setting disabled state once (e.g., in the component’s `ngOnInit`/`prePopulate`, or by configuring the Formly field definitions up-front) rather than mutating fields on every render.
```suggestion
// 3) Enforce at the reactive form level when the control already exists.
if (field.formControl) {
if (disabled) {
field.formControl.disable({ emitEvent: false });
} else {
field.formControl.enable({ emitEvent: false });
}
```
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/tuple/Attribute.java:
##########
@@ -21,6 +21,10 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.texera.amber.pybuilder.EncodableStringAnnotation;
+import org.apache.texera.amber.pybuilder.PyStringTypes;
+import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableStringFactory$;
Review Comment:
The new imports `PyStringTypes` and `PyStringTypes.EncodableStringFactory$` are unused in this file. In Java, unused imports cause a compilation error, so the build will fail unless these are removed (only `EncodableStringAnnotation` is referenced below).
```suggestion
```
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
Review Comment:
`buildInjectedParametersMap` splices `parameter.attribute.getName()` into a single-quoted Python string literal via Scala interpolation. This bypasses the pybuilder encoding/escaping (and defeats the new `@EncodableStringAnnotation` on `Attribute.getName()`), so attribute names containing quotes/backslashes can generate invalid Python. Prefer rendering the key as a Python expression (e.g., via `pyb"${parameter.attribute.getName()}: ..."`) or otherwise ensure proper escaping rather than manual `'${...}'` quoting.
```suggestion
pyb"${parameter.attribute.getName()}: ${parameter.value}"
```
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4437716804
@carloea2 Thanks for sharing your thoughts. We are all busy with a coming hackathon, and will get back to you after that.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025313954
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
+ }
+
+ entries.reduceOption((acc, entry) => acc + pyb", " + entry).getOrElse(pyb"")
+ }
+
+ private def buildInjectedHookMethod(uiParameters: List[UiUDFParameter]): String = {
+ val injectedParametersMap = buildInjectedParametersMap(uiParameters)
+
+ // unindented method; we indent it when inserting into the class body
+ (pyb"""|@overrides
+ |def """ + pyb"$ReservedHookMethod" + pyb"""(self) -> typing.Dict[str, typing.Any]:
+ | return {""" +
+ injectedParametersMap +
+ pyb"""}
+ |""").encode
+ }
+
+ private def ensureTypingImport(encodedUserCode: String): String = {
+ val alreadyImported = encodedUserCode
+ .split("\n", -1)
+ .exists(_.trim == "import typing")
+
+ if (alreadyImported) {
+ encodedUserCode
+ } else {
+ "import typing\n" + encodedUserCode
+ }
Review Comment:
Good idea but too complex
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4187616849
> Left some comments. The PR looks more polished.
>
> Can we break this down into smaller PRs? This PR is too large. For example, can we make a separate PR to fix the share-editing issue? Is this feature also supported for R UDF and Java UDF? If so, can we split the R UDF and the Java UDF into separate PRs?
The share editing issue has been merged as separate PR, this PR only adds support for python.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r2907501114
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts:
##########
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service";
+
+describe("UiUdfParametersParserService", () => {
+ let service: UiUdfParametersParserService;
+
+ beforeEach(() => {
+ service = new UiUdfParametersParserService();
+ });
+
+ it("should parse positional and name-based arguments", () => {
+ const code = `
+ class ProcessTupleOperator(UDFOperatorV2):
+ def open(self):
+ self.UiParameter(AttributeType.INT, "count")
+ self.UiParameter(type=AttributeType.STRING, name="name")
+ self.UiParameter(name="age", type=AttributeType.LONG)
+ self.UiParameter(AttributeType.DOUBLE, name="score")
+ self.UiParameter("created_at", type=AttributeType.TIMESTAMP)
+ `;
+
+ expect(service.parse(code)).toEqual([
+ { attribute: { attributeName: "count", attributeType: "INT" }, value: "" },
+ { attribute: { attributeName: "name", attributeType: "STRING" }, value: "" },
+ { attribute: { attributeName: "age", attributeType: "LONG" }, value: "" },
+ { attribute: { attributeName: "score", attributeType: "DOUBLE" }, value: "" },
+ { attribute: { attributeName: "created_at", attributeType: "TIMESTAMP" }, value: "" },
+ ]);
Review Comment:
The test expectations use raw Python type token names (e.g., `"INT"`, `"STRING"`, `"LONG"`, `"DOUBLE"`, `"TIMESTAMP"`) but the `UiUdfParametersParserService.parse()` method normalizes these tokens to canonical schema names (e.g., `"integer"`, `"string"`, `"long"`, `"double"`, `"timestamp"`) via `normalizeAttributeType()`. The correct expected values should be `"integer"` for `INT`, `"string"` for `STRING`, `"long"` for `LONG`, `"double"` for `DOUBLE`, and `"timestamp"` for `TIMESTAMP`. As written, this test will fail.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4302009060
@kunwp1 ?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4230987488
@aglinxinyuan please continue with the review
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105652834
##########
amber/src/main/python/pyamber/__init__.py:
##########
Review Comment:
Yes you are right for this case.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105746356
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,435 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import {
+ AttributeType,
+ JavaAttributeTypeName,
+ PythonAttributeTypeName,
+ SchemaAttribute,
+ JAVA_ATTRIBUTE_TYPE_NAMES,
+ PYTHON_ATTRIBUTE_TYPE_NAMES,
+} from "../../types/workflow-compiling.interface";
+
+export interface UiUdfParameter {
+ attribute: SchemaAttribute;
+ value: string;
+}
+
+type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName;
+
+const ATTRIBUTE_TYPE_TOKEN_TO_CANONICAL: Readonly<Record<ParserAttributeTypeToken, AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+ BINARY: "binary",
+ LARGE_BINARY: "large_binary",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "string",
+ "integer",
+ "long",
+ "double",
+ "boolean",
+ "timestamp",
+]);
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersParserService {
+ private static readonly SUPPORTED_CLASSES = [
+ "ProcessTupleOperator",
+ "ProcessBatchOperator",
+ "ProcessTableOperator",
+ "GenerateOperator",
+ ];
+
+ parse(code: string): UiUdfParameter[] {
+ if (!code) {
+ return [];
+ }
+
+ const sanitizedCode = this.stripCommentsAndDocstrings(code);
+ const classPattern = UiUdfParametersParserService.SUPPORTED_CLASSES.join("|");
+ const classRegex = new RegExp(
+ `class\\s+(${classPattern})\\s*\\([^)]*\\)\\s*:[\\s\\S]*?(?=\\nclass\\s+\\w+\\s*\\(|$)`,
+ "g"
+ );
+
+ const parsed: UiUdfParameter[] = [];
+ const existingNames = new Set<string>();
+
+ let classMatch: RegExpExecArray | null;
+ while ((classMatch = classRegex.exec(sanitizedCode)) !== null) {
+ const classBlock = classMatch[0];
+
+ for (const args of this.extractUiParameterArgumentLists(classBlock)) {
+ const argumentTokens = this.tokenizeArguments(args);
+ const extracted = this.extractParameter(argumentTokens);
+ if (!extracted || existingNames.has(extracted.attribute.attributeName)) {
+ continue;
+ }
+
+ existingNames.add(extracted.attribute.attributeName);
+ parsed.push(extracted);
+ }
+ }
+
+ return parsed;
+ }
+
+ private stripCommentsAndDocstrings(code: string): string {
Review Comment:
Done. Previous 400LOC new is 200LOC
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4435830678
I want to share some feedback about the review process for apache/texera. This is not meant to reopen the PR or attack anyone personally, but I do think the experience raised some process concerns that I want to share.
This PR was +1,737 / -19. Around the same time, larger PRs were merged, for example:
1. apache/texera#4540: +2,476 / -10
2. apache/texera#4331: +1,888 / -384
3. apache/texera#4495: +9,473 / -2,774
Because of that, I found it difficult to understand/trust how PR size and review expectations are being applied consistently.
I also do not think Copilot reviews should carry much weight without human validation. In this PR, Copilot hallucinated at least twice. AI review can be useful as an extra signal, but it should not become a substitute for timely human review, especially when a PR is being delayed or blocked.
More generally, my experience contributing has become increasingly time-consuming and less pleasant. Review cycles feel longer, there are idle periods, and it is not always clear what is expected from the contributor. That makes the process frustrating, especially when feedback comes slowly or mainly from automated tools.
I also want to point out that test-related PRs should be reviewed with the same seriousness as other PRs. I do not agree with the idea that test PRs are lower-risk or that weaker review is acceptable because they are “only tests.” Poor tests can still create maintenance burden, false confidence, and long-term quality issues.
If reviewers have the right to reject a PR for quality reasons, then contributors should also be able to expect timely, consistent, and meaningful human review. Otherwise, the process becomes unbalanced: contributors are expected to respond carefully to every comment, while automated review can generate unreliable or redundant feedback without the same level of accountability.
I am sharing this as my personal perspective based on this PR and nearby examples. I am not asking everyone to agree with me and I do not intend to defend this perspective, but I do think the review process could be more consistent, transparent, and respectful of contributor time.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105490869
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,435 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import {
+ AttributeType,
+ JavaAttributeTypeName,
+ PythonAttributeTypeName,
+ SchemaAttribute,
+ JAVA_ATTRIBUTE_TYPE_NAMES,
+ PYTHON_ATTRIBUTE_TYPE_NAMES,
+} from "../../types/workflow-compiling.interface";
+
+export interface UiUdfParameter {
+ attribute: SchemaAttribute;
+ value: string;
+}
+
+type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName;
+
+const ATTRIBUTE_TYPE_TOKEN_TO_CANONICAL: Readonly<Record<ParserAttributeTypeToken, AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+ BINARY: "binary",
+ LARGE_BINARY: "large_binary",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "string",
+ "integer",
+ "long",
+ "double",
+ "boolean",
+ "timestamp",
+]);
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersParserService {
+ private static readonly SUPPORTED_CLASSES = [
+ "ProcessTupleOperator",
+ "ProcessBatchOperator",
+ "ProcessTableOperator",
+ "GenerateOperator",
+ ];
+
+ parse(code: string): UiUdfParameter[] {
+ if (!code) {
+ return [];
+ }
+
+ const sanitizedCode = this.stripCommentsAndDocstrings(code);
+ const classPattern = UiUdfParametersParserService.SUPPORTED_CLASSES.join("|");
+ const classRegex = new RegExp(
+ `class\\s+(${classPattern})\\s*\\([^)]*\\)\\s*:[\\s\\S]*?(?=\\nclass\\s+\\w+\\s*\\(|$)`,
+ "g"
+ );
+
+ const parsed: UiUdfParameter[] = [];
+ const existingNames = new Set<string>();
+
+ let classMatch: RegExpExecArray | null;
+ while ((classMatch = classRegex.exec(sanitizedCode)) !== null) {
+ const classBlock = classMatch[0];
+
+ for (const args of this.extractUiParameterArgumentLists(classBlock)) {
+ const argumentTokens = this.tokenizeArguments(args);
+ const extracted = this.extractParameter(argumentTokens);
+ if (!extracted || existingNames.has(extracted.attribute.attributeName)) {
+ continue;
+ }
+
+ existingNames.add(extracted.attribute.attributeName);
+ parsed.push(extracted);
+ }
+ }
+
+ return parsed;
+ }
+
+ private stripCommentsAndDocstrings(code: string): string {
Review Comment:
Please find it by yourself first and let me know if you cannot find one.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105490454
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/DualInputPortsPythonUDFOpDescV2.scala:
##########
@@ -34,11 +34,25 @@ class DualInputPortsPythonUDFOpDescV2 extends LogicalOp {
required = true,
defaultValue =
"# Choose from the following templates:\n" +
+ "# \n" +
+ "# UiParameter notes:\n" +
+ "# - A UiParameter is a user-editable value exposed in the property panel and read from your Python code.\n" +
+ "# - Define UiParameter values in open() and then use them later in your UDF methods.\n" +
+ "# - Active UiParameter calls appear in the property panel; commented-out calls are ignored.\n" +
+ "# - Supported UiParameter types are STRING, INT/LONG, DOUBLE, BOOL, and TIMESTAMP.\n" +
"# \n" +
"# from pytexera import *\n" +
"# \n" +
"# class ProcessTupleOperator(UDFOperatorV2):\n" +
- "# \n" +
+ "# @overrides\n" +
Review Comment:
I don't see the need of having three different copies as they are all using the same APIs. Please move the comment on the top and remove duplicates.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105568143
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
Review Comment:
I think it should stay in the same place, ui-udf-parameters-parser.service.ts is not redefining them or overriding them.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105745031
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
2) New behaviour:
"" -> False
" " -> False
"True" -> True
"true" -> True
"1" -> True
"1.0" -> True
"2" -> True
"-1" -> True
"False" -> False
"false" -> False
"0" -> False
"0.0" -> False
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4173997470
@aglinxinyuan can you give it a pass? Thanks
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4249835766
@chenlica I will fix it after the current code pass the review, that is more efficient I think.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4249910536
> > @chenlica I will fix it after the current code pass the review, that is more efficient I think.
>
> I just did the fix since it's caused by my changes. It's usually easier to resolve conflicts earlier than later.
Ok, thanks.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4272826451
@kunwp1 thanks for the review, I will let you know when the diagram is ready, though I think it may not reflect the changes, since the architecture is the same, just changed the way the UDFs contents are generated.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025301310
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
+ }
+
+ entries.reduceOption((acc, entry) => acc + pyb", " + entry).getOrElse(pyb"")
+ }
+
+ private def buildInjectedHookMethod(uiParameters: List[UiUDFParameter]): String = {
+ val injectedParametersMap = buildInjectedParametersMap(uiParameters)
+
+ // unindented method; we indent it when inserting into the class body
+ (pyb"""|@overrides
+ |def """ + pyb"$ReservedHookMethod" + pyb"""(self) -> typing.Dict[str, typing.Any]:
+ | return {""" +
+ injectedParametersMap +
+ pyb"""}
+ |""").encode
Review Comment:
Default UDF template already uses override, no need for this.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025323869
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
+ }
+
+ entries.reduceOption((acc, entry) => acc + pyb", " + entry).getOrElse(pyb"")
+ }
+
+ private def buildInjectedHookMethod(uiParameters: List[UiUDFParameter]): String = {
+ val injectedParametersMap = buildInjectedParametersMap(uiParameters)
+
+ // unindented method; we indent it when inserting into the class body
+ (pyb"""|@overrides
+ |def """ + pyb"$ReservedHookMethod" + pyb"""(self) -> typing.Dict[str, typing.Any]:
+ | return {""" +
+ injectedParametersMap +
+ pyb"""}
+ |""").encode
+ }
+
+ private def ensureTypingImport(encodedUserCode: String): String = {
+ val alreadyImported = encodedUserCode
+ .split("\n", -1)
+ .exists(_.trim == "import typing")
+
+ if (alreadyImported) {
+ encodedUserCode
+ } else {
+ "import typing\n" + encodedUserCode
+ }
Review Comment:
I am going to add the needed imports under pytexera directly and stop injecting them in the UDF.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105598747
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
Review Comment:
It looks like JAVA_ATTRIBUTE_TYPE_NAMES, JavaAttributeTypeName, and the other related tokens defined in `workflow-compiling-interface.ts` are only used within `ui-udf-parameters-parser.service.ts`. To better follow the principle of colocation and reduce unnecessary cross-file dependencies, I suggest we move these definitions directly into the service.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105612224
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
Review Comment:
Ok
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105671398
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,435 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import {
+ AttributeType,
+ JavaAttributeTypeName,
+ PythonAttributeTypeName,
+ SchemaAttribute,
+ JAVA_ATTRIBUTE_TYPE_NAMES,
+ PYTHON_ATTRIBUTE_TYPE_NAMES,
+} from "../../types/workflow-compiling.interface";
+
+export interface UiUdfParameter {
+ attribute: SchemaAttribute;
+ value: string;
+}
+
+type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName;
+
+const ATTRIBUTE_TYPE_TOKEN_TO_CANONICAL: Readonly<Record<ParserAttributeTypeToken, AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+ BINARY: "binary",
+ LARGE_BINARY: "large_binary",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "string",
+ "integer",
+ "long",
+ "double",
+ "boolean",
+ "timestamp",
+]);
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersParserService {
+ private static readonly SUPPORTED_CLASSES = [
+ "ProcessTupleOperator",
+ "ProcessBatchOperator",
+ "ProcessTableOperator",
+ "GenerateOperator",
+ ];
+
+ parse(code: string): UiUdfParameter[] {
+ if (!code) {
+ return [];
+ }
+
+ const sanitizedCode = this.stripCommentsAndDocstrings(code);
+ const classPattern = UiUdfParametersParserService.SUPPORTED_CLASSES.join("|");
+ const classRegex = new RegExp(
+ `class\\s+(${classPattern})\\s*\\([^)]*\\)\\s*:[\\s\\S]*?(?=\\nclass\\s+\\w+\\s*\\(|$)`,
+ "g"
+ );
+
+ const parsed: UiUdfParameter[] = [];
+ const existingNames = new Set<string>();
+
+ let classMatch: RegExpExecArray | null;
+ while ((classMatch = classRegex.exec(sanitizedCode)) !== null) {
+ const classBlock = classMatch[0];
+
+ for (const args of this.extractUiParameterArgumentLists(classBlock)) {
+ const argumentTokens = this.tokenizeArguments(args);
+ const extracted = this.extractParameter(argumentTokens);
+ if (!extracted || existingNames.has(extracted.attribute.attributeName)) {
+ continue;
+ }
+
+ existingNames.add(extracted.attribute.attributeName);
+ parsed.push(extracted);
+ }
+ }
+
+ return parsed;
+ }
+
+ private stripCommentsAndDocstrings(code: string): string {
Review Comment:
ChatGPT recommneded: @lezer/python, I will give it a try.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105489136
##########
amber/src/main/python/pyamber/__init__.py:
##########
Review Comment:
I don't think this is correct. You are already importing it on pytexera. I don't see the need of adding it on pyamber. Please test it by yourself and leave comments.
##########
amber/src/main/python/pytexera/__init__.py:
##########
@@ -53,4 +54,7 @@
"Iterator",
"Optional",
"Union",
+ "Dict",
Review Comment:
Same here.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105565729
##########
amber/src/main/python/pyamber/__init__.py:
##########
Review Comment:
Ok
##########
amber/src/main/python/pytexera/__init__.py:
##########
@@ -53,4 +54,7 @@
"Iterator",
"Optional",
"Union",
+ "Dict",
Review Comment:
Ok
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/wordCloud/WordCloudOpDesc.scala:
##########
Review Comment:
Thanks
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105522095
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
Review Comment:
Then I don't see the need of putting it under this file as this file is used for common type usages. Please move the definition to `ui-udf-parameters-parser.service.ts`
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105537038
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
Also please check how the text input scan operator is implemented.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4274200528
I haven't read the details of the diagram yet but will do it later after all the comments are addressed. Please move the diagrams to the PR description.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105626984
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
1) Done.
2) They use this on java side:
https://github.com/apache/texera/blob/ab3317b8ef12e1b56205bf0f35c5ae1edde42d9d/common/workflow-core/src/main/scala/org/apache/texera/amber/core/tuple/AttributeTypeUtils.scala#L259-L283
I will update the parser to do exactly the same.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4272882772
@kunwp1 are these diagrams ok?:
Overall:
```mermaid
sequenceDiagram
autonumber
participant FE as Frontend UI
participant Parser as Parser Service
participant Store as Operator Properties
participant BE as Scala Backend
participant Inj as Python Code Injector
participant Py as Python Runtime
participant UDF as User UDF
FE->>Parser: Send Python code
Parser->>Parser: Extract self.UiParameter declarations
Parser-->>FE: Return parameter schema
FE->>Store: Save uiParameters<br/>name, type, value
Store->>BE: Submit operator descriptor
BE->>Inj: Pass original code + uiParameters
Inj->>Inj: Validate names and types
Inj->>Inj: Generate injected hook
Inj-->>BE: Return modified Python code
BE->>Py: Execute injected code
Py->>UDF: Load UDF class
UDF->>Py: open() is wrapped
Py->>UDF: Call injected hook
UDF-->>Py: Return parameter values
Py->>UDF: Provide typed UiParameter values
UDF->>Py: Execute workflow logic
```
Detailed:
```mermaid
sequenceDiagram
autonumber
actor User
participant CodeEditor as CodeEditorComponent<br/>Monaco editor
participant YCode as Yjs shared code
participant Sync as UiUdfParametersSyncService
participant Parser as UiUdfParametersParserService
participant ParamUI as UiUdfParametersComponent
participant Formly as Formly / Operator Properties
participant OpDesc as Python UDF Operator Descriptor<br/>Scala backend
participant Injector as PythonUdfUiParameterInjector
participant Runtime as Python Runtime
participant Support as _UiParameterSupport
participant UDF as User UDF Class
User->>CodeEditor: Writes Python UDF code
CodeEditor->>YCode: Updates shared code text
YCode->>Sync: Emits code-change event
Sync->>Parser: Parse latest Python code
Parser->>Parser: Find supported UDF class
Parser->>Parser: Scan self.UiParameter(...)
Parser-->>Sync: Return inferred parameters<br/>name + AttributeType
Sync->>Formly: Update operator uiParameters structure
Formly->>ParamUI: Render parameter rows
ParamUI-->>User: Shows inferred params<br/>name/type locked, value editable
User->>ParamUI: Enters parameter values
ParamUI->>Formly: Store edited values
Formly->>OpDesc: Persist uiParameters in operator properties
User->>OpDesc: Runs workflow
OpDesc->>Injector: inject(code, uiParameters)
Injector->>Injector: Validate uiParameters
Injector->>Injector: Build parameter map
Injector->>Injector: Generate reserved hook method
Injector->>Injector: Insert hook into user UDF class
Injector-->>OpDesc: Return injected Python code
OpDesc->>Runtime: Execute OpExecWithCode(injectedCode, "python")
Runtime->>UDF: Load user UDF class
UDF->>Support: __init_subclass__ wraps open()
Runtime->>UDF: Start operator execution
UDF->>Support: wrapped open() runs first
Support->>UDF: Call _texera_injected_ui_parameters()
UDF-->>Support: Return UI parameter values
Support->>Support: Store injected values
Support->>UDF: Continue original user open()
UDF->>Support: Create/read self.UiParameter(...)
Support->>Support: Look up injected value by name
Support->>Support: Convert value using AttributeType
Support-->>UDF: Return typed Python value
UDF->>Runtime: Process tuples / batches / source output
```
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3103082791
##########
amber/src/main/python/pyamber/__init__.py:
##########
Review Comment:
I don't think you have to update this file. Can you revert this?
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,435 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import {
+ AttributeType,
+ JavaAttributeTypeName,
+ PythonAttributeTypeName,
+ SchemaAttribute,
+ JAVA_ATTRIBUTE_TYPE_NAMES,
+ PYTHON_ATTRIBUTE_TYPE_NAMES,
+} from "../../types/workflow-compiling.interface";
+
+export interface UiUdfParameter {
+ attribute: SchemaAttribute;
+ value: string;
+}
+
+type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName;
+
+const ATTRIBUTE_TYPE_TOKEN_TO_CANONICAL: Readonly<Record<ParserAttributeTypeToken, AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+ BINARY: "binary",
+ LARGE_BINARY: "large_binary",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "string",
+ "integer",
+ "long",
+ "double",
+ "boolean",
+ "timestamp",
+]);
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersParserService {
+ private static readonly SUPPORTED_CLASSES = [
+ "ProcessTupleOperator",
+ "ProcessBatchOperator",
+ "ProcessTableOperator",
+ "GenerateOperator",
+ ];
+
+ parse(code: string): UiUdfParameter[] {
+ if (!code) {
+ return [];
+ }
+
+ const sanitizedCode = this.stripCommentsAndDocstrings(code);
+ const classPattern = UiUdfParametersParserService.SUPPORTED_CLASSES.join("|");
+ const classRegex = new RegExp(
+ `class\\s+(${classPattern})\\s*\\([^)]*\\)\\s*:[\\s\\S]*?(?=\\nclass\\s+\\w+\\s*\\(|$)`,
+ "g"
+ );
+
+ const parsed: UiUdfParameter[] = [];
+ const existingNames = new Set<string>();
+
+ let classMatch: RegExpExecArray | null;
+ while ((classMatch = classRegex.exec(sanitizedCode)) !== null) {
+ const classBlock = classMatch[0];
+
+ for (const args of this.extractUiParameterArgumentLists(classBlock)) {
+ const argumentTokens = this.tokenizeArguments(args);
+ const extracted = this.extractParameter(argumentTokens);
+ if (!extracted || existingNames.has(extracted.attribute.attributeName)) {
+ continue;
+ }
+
+ existingNames.add(extracted.attribute.attributeName);
+ parsed.push(extracted);
+ }
+ }
+
+ return parsed;
+ }
+
+ private stripCommentsAndDocstrings(code: string): string {
Review Comment:
The logics you implemented here looks too trivial. Did you check if there are existing third-party libraries that does this?
##########
amber/src/main/python/pytexera/__init__.py:
##########
@@ -53,4 +54,7 @@
"Iterator",
"Optional",
"Union",
+ "Dict",
Review Comment:
Remove Dict and Any
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
Review Comment:
Why does this PR include JAVA_ATTRIBUTE_TYPE_NAMES? Isn't this PR only for Python UDF?
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/wordCloud/WordCloudOpDesc.scala:
##########
Review Comment:
Revert unnecessary changes.
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
How did you come up with these three values ("true", "1", "yes")? Do you think it's okay to return false if the value is 2 for example?
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/DualInputPortsPythonUDFOpDescV2.scala:
##########
@@ -34,11 +34,25 @@ class DualInputPortsPythonUDFOpDescV2 extends LogicalOp {
required = true,
defaultValue =
"# Choose from the following templates:\n" +
+ "# \n" +
+ "# UiParameter notes:\n" +
+ "# - A UiParameter is a user-editable value exposed in the property panel and read from your Python code.\n" +
+ "# - Define UiParameter values in open() and then use them later in your UDF methods.\n" +
+ "# - Active UiParameter calls appear in the property panel; commented-out calls are ignored.\n" +
+ "# - Supported UiParameter types are STRING, INT/LONG, DOUBLE, BOOL, and TIMESTAMP.\n" +
"# \n" +
"# from pytexera import *\n" +
"# \n" +
"# class ProcessTupleOperator(UDFOperatorV2):\n" +
- "# \n" +
+ "# @overrides\n" +
Review Comment:
You might only need one copy of the comment.
##########
frontend/src/app/common/formly/collab-wrapper/collab-wrapper/collab-wrapper.component.css:
##########
Review Comment:
Revert this change.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105487932
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
Then I suggest to use a dropdown menu for boolean type to only have two options.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105585399
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
1. Could you create an issue to track this task?
2. The Text Input Scan already implements a similar feature where users can specify a text input to be passed as a boolean to downstream operators. I suggest following that implementation as a reference.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4256054888
@kunwp1
I removed support for Binary/Large Binary
I took care of the parsing issue
I added doc blocks to the UDF, let me know if you want a change.
Thanks.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4173720992
@aglinxinyuan Can you request a new copilot review?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4173724697
> @aglinxinyuan Can you request a new copilot review?
done.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025299432
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,189 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
Review Comment:
Good.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025306367
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,17 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: int,
+ AttributeType.LONG: int,
+ AttributeType.DOUBLE: float,
+ AttributeType.BOOL: lambda v: str(v).strip().lower() in ("true", "1", "yes"),
+ AttributeType.BINARY: lambda v: v if isinstance(v, bytes) else str(v).encode(),
+ AttributeType.TIMESTAMP: lambda v: datetime.datetime.fromisoformat(v),
+ AttributeType.LARGE_BINARY: largebinary,
+}
Review Comment:
Timestamps was tested and working, no need to change it.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4274463007
@kunwp1
Thanks. I have resolved your comments. The diagram is also now in the PR description.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104561696
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
It was in the effort of a best try attempt, no special reason, we can go with the plan you think is better.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104564617
##########
amber/src/main/python/pytexera/__init__.py:
##########
@@ -53,4 +54,7 @@
"Iterator",
"Optional",
"Union",
+ "Dict",
Review Comment:
No, this change is needed for the feature to work. The reason is the definitions of UiParameter use those as the return value types, so since the code comes from the User, I make them available from the import.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4306105468
It's another example to show the importance to have small PRs.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4306141130
I think someone from outside wanting to contribute will feel somewhat frustrated after so much time and so many messages, only to have to close the PR. Do the Texera guides have any recommendations for the number of lines of code?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4301647087
@kunwp1 Hi do you have any update about the PR?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4016794468
@aglinxinyuan Please review it.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r2907501231
##########
amber/src/main/python/pytexera/udf/udf_operator.py:
##########
@@ -16,12 +16,76 @@
# under the License.
from abc import abstractmethod
-from typing import Iterator, Optional, Union
-
-from pyamber import *
+from typing import Any, Dict, Iterator, Optional, Union
+import functools
-class UDFOperatorV2(TupleOperatorV2):
+from pyamber import *
+from core.models.schema.attribute_type import AttributeType, FROM_STRING_PARSER_MAPPING
+
+class _UiParameterSupport:
+ _ui_parameter_injected_values: Dict[str, Any] = {}
+ _ui_parameter_name_types: Dict[str, AttributeType] = {}
+
+ # Reserved hook name. Backend injector will generate this in the user's class.
+ def _texera_injected_ui_parameters(self) -> Dict[str, Any]:
+ return {}
+
+ def _texera_apply_injected_ui_parameters(self) -> None:
+ values = self._texera_injected_ui_parameters()
+ # Write to base class storage (not cls) because UiParameter reads from _UiParameterSupport directly
+ _UiParameterSupport._ui_parameter_injected_values = dict(values or {})
+ _UiParameterSupport._ui_parameter_name_types = {}
Review Comment:
`_ui_parameter_injected_values` and `_ui_parameter_name_types` are class-level dictionaries on `_UiParameterSupport` (lines 27-28). This means all subclass instances share the same state. If multiple UDF operator instances run concurrently in the same Python worker process (e.g., due to parallelism), one operator's `_texera_apply_injected_ui_parameters()` call could overwrite another operator's injected values, causing incorrect parameter values to be read. Each operator instance should have its own storage, for example by using instance-level variables instead of class-level ones.
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.spec.ts:
##########
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { UiUdfParametersParserService } from "./ui-udf-parameters-parser.service";
+
+describe("UiUdfParametersParserService", () => {
+ let service: UiUdfParametersParserService;
+
+ beforeEach(() => {
+ service = new UiUdfParametersParserService();
+ });
+
+ it("should parse positional and name-based arguments", () => {
+ const code = `
+ class ProcessTupleOperator(UDFOperatorV2):
+ def open(self):
+ self.UiParameter(AttributeType.INT, "count")
+ self.UiParameter(type=AttributeType.STRING, name="name")
+ self.UiParameter(name="age", type=AttributeType.LONG)
+ self.UiParameter(AttributeType.DOUBLE, name="score")
+ self.UiParameter("created_at", type=AttributeType.TIMESTAMP)
+ `;
Review Comment:
There is a mismatch between the test code calling convention and the actual Python `UiParameter` signature. The test uses `self.UiParameter(AttributeType.INT, "count")` — type first, name second. However, the Python `UiParameter.__init__` signature is `def __init__(self, name: str, type: AttributeType)` (name first, type second). If a user follows this positional call pattern from the test, calling `self.UiParameter(AttributeType.INT, "count")` in Python would fail with a `TypeError` since `AttributeType.INT` would be passed as `name` and `"count"` as `type`. While the frontend parser correctly handles both orderings (since it detects positional tokens by pattern matching), the test inadvertently documents an unsupported Python call pattern. The test should use the correct Python positional call convention, or strictly use keyword arguments.
##########
frontend/src/app/workspace/component/ui-udf-parameters/ui-udf-parameters.component.html:
##########
@@ -0,0 +1,55 @@
+<!--
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+-->
+<div
+ class="ui-udf-param-list"
+ *ngIf="(model?.length ?? 0) > 0">
+ <!-- Optional header row -->
+ <div class="ui-udf-param-row header">
+ <div class="field-cell"><span class="col-title">Value</span></div>
+ <div class="field-cell"><span class="col-title">Name</span></div>
+ <div class="field-cell"><span class="col-title">Type</span></div>
+ </div>
+
+ <div
+ class="ui-udf-param-row"
+ *ngFor="let param of (model || []); let i = index; trackBy: trackByParamName">
+ <ng-container *ngIf="field.fieldGroup?.[i] as rowField">
+ <!-- Value -->
+ <div class="field-cell">
+ <ng-container *ngIf="getValueField(rowField) as valueField">
+ <formly-field [field]="valueField"></formly-field>
+ </ng-container>
+ </div>
+
+ <!-- Name -->
+ <div class="field-cell">
+ <ng-container *ngIf="getNameField(rowField) as nameField">
+ <formly-field [field]="nameField"></formly-field>
+ </ng-container>
+ </div>
+
+ <!-- Type -->
+ <div class="field-cell">
+ <ng-container *ngIf="getTypeField(rowField) as typeField">
+ <formly-field [field]="typeField"></formly-field>
+ </ng-container>
+ </div>
+ </ng-container>
+ </div>
+</div>
Review Comment:
The column order in the UI template renders "Value", "Name", "Type" (left to right) across both the header row and data rows. However, the PR description screenshot shows the column order as "Name", "Type", "Value". This is a discrepancy between the PR description and the actual code. The PR description screenshot likely shows the intended UX (Name and Type are read-only attributes that describe the parameter, while Value is what users edit). The current code places the editable "Value" column first, which may be confusing since users typically scan left-to-right and seeing Value before knowing the Name/Type of the parameter is less intuitive.
##########
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts:
##########
@@ -47,11 +47,12 @@ import "@codingame/monaco-vscode-python-default-extension";
import "@codingame/monaco-vscode-r-default-extension";
import "@codingame/monaco-vscode-java-default-extension";
import { isDefined } from "../../../common/util/predicate";
-import { filter, switchMap } from "rxjs/operators";
+import { debounceTime, filter, switchMap } from "rxjs/operators";
Review Comment:
`debounceTime` is imported but never used in this file. The commented-out `pythonCodeChangeSubject` field on line 106 suggests this was intended for throttling code change events, but the implementation was changed to use `yCode.observe(handler)` directly without debouncing. The unused import should be removed to keep the code clean.
##########
frontend/src/app/common/formly/collab-wrapper/collab-wrapper/collab-wrapper.component.css:
##########
@@ -25,7 +25,7 @@
outline: transparent;
overflow: visible;
position: relative;
- : 1pt;
+ :1pt;
Review Comment:
The CSS rule on line 28 is invalid: `:1pt;` is not a valid CSS declaration — it has no property name. This appears to be a stray/broken rule. The original code before this change was ` : 1pt;` which was also invalid, but the refactoring of removing the leading space didn't fix it. This line should either be removed entirely or corrected to a valid CSS property (e.g., `border: 1pt;`).
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjector.scala:
##########
@@ -0,0 +1,196 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder
+import org.apache.texera.amber.pybuilder.PythonTemplateBuilder.PythonTemplateBuilderStringContext
+
+import scala.util.matching.Regex
+
+object PythonUdfUiParameterInjector {
+
+ private val ReservedHookMethod = "_texera_injected_ui_parameters"
+
+ // Match user-facing UDF classes (the ones users write)
+ private val SupportedUserClassRegex: Regex =
+ """(?m)^([ \t]*)class\s+(ProcessTupleOperator|ProcessBatchOperator|ProcessTableOperator|GenerateOperator)\s*\([^)]*\)\s*:\s*(?:#.*)?$""".r
+
+ private def validate(uiParameters: List[UiUDFParameter]): Unit = {
+ uiParameters.foreach { parameter =>
+ if (parameter.attribute == null) {
+ throw new RuntimeException("UiParameter attribute is required.")
+ }
+ }
+
+ val grouped = uiParameters.groupBy(_.attribute.getName)
+ grouped.foreach {
+ case (key, values) =>
+ val typeSet = values.map(_.attribute.getType).toSet
+ if (typeSet.size > 1) {
+ throw new RuntimeException(
+ s"UiParameter key '$key' has multiple types: ${typeSet.map(_.name()).mkString(",")}."
+ )
+ }
+ }
+ }
+
+ private def buildInjectedParametersMap(
+ uiParameters: List[UiUDFParameter]
+ ): PythonTemplateBuilder = {
+ val entries = uiParameters.map { parameter =>
+ pyb"'${parameter.attribute.getName()}': ${parameter.value}"
+ }
+
+ entries.reduceOption((acc, entry) => acc + pyb", " + entry).getOrElse(pyb"")
+ }
+
+ private def buildInjectedHookMethod(uiParameters: List[UiUDFParameter]): String = {
+ val injectedParametersMap = buildInjectedParametersMap(uiParameters)
+
+ // unindented method; we indent it when inserting into the class body
+ (pyb"""|@overrides
+ |def """ + pyb"$ReservedHookMethod" + pyb"""(self) -> typing.Dict[str, typing.Any]:
+ | return {""" +
+ injectedParametersMap +
+ pyb"""}
+ |""").encode
+ }
+
+ private def ensureTypingImport(encodedUserCode: String): String = {
+ val alreadyImported = encodedUserCode
+ .split("\n", -1)
+ .exists(_.trim == "import typing")
+
+ if (alreadyImported) {
+ encodedUserCode
+ } else {
+ "import typing\n" + encodedUserCode
+ }
+ }
+
+ private def indentBlock(block: String, indent: String): String = {
+ block
+ .split("\n", -1)
+ .map { line =>
+ if (line.nonEmpty) indent + line else line
+ }
+ .mkString("\n")
+ }
+
+ private def lineEndIndex(text: String, from: Int): Int = {
+ val idx = text.indexOf('\n', from)
+ if (idx < 0) text.length else idx
+ }
+
+ private def detectClassBlockEnd(code: String, classHeaderStart: Int, classIndent: String): Int = {
+ val classLineEnd = lineEndIndex(code, classHeaderStart)
+ var pos = if (classLineEnd < code.length) classLineEnd + 1 else code.length
+
+ while (pos < code.length) {
+ val end = lineEndIndex(code, pos)
+ val line = code.substring(pos, end)
+
+ val trimmed = line.trim
+ val isBlank = trimmed.isEmpty
+
+ // a top-level (or same/lower-indented) non-blank line ends the class block
+ val currentIndentLen = line.prefixLength(ch => ch == ' ' || ch == '\t')
+ val classIndentLen = classIndent.length
+
+ if (!isBlank && currentIndentLen <= classIndentLen) {
+ return pos
+ }
+
+ pos = if (end < code.length) end + 1 else code.length
+ }
+
+ code.length
+ }
+
+ private def containsReservedHook(classBlock: String): Boolean = {
+ val hookRegex = ("""(?m)^[ \t]+def\s+""" + Regex.quote(ReservedHookMethod) + """\s*\(""").r
+ hookRegex.findFirstIn(classBlock).isDefined
+ }
+
+ private def findInsertionPointInsideClass(classBlock: String, classIndent: String): Int = {
+ // Insert before the first method definition in the class body.
+ // This preserves existing open() and also preserves class docstrings if present.
+ val methodRegex = """(?m)^[ \t]+def\s+\w+\s*\(""".r
+ methodRegex.findFirstMatchIn(classBlock).map(_.start).getOrElse(classBlock.length)
+ }
Review Comment:
The `findInsertionPointInsideClass` method is defined on line 131 but is never called anywhere in the file. The `injectHookIntoUserClass` method instead always appends the injected hook at the end of the class block (`detectClassBlockEnd` is used) rather than before the first method. This dead method should be removed to avoid confusion.
##########
common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/udf/python/PythonUdfUiParameterInjectorSpec.scala:
##########
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.texera.amber.operator.udf.python
+
+import org.apache.texera.amber.core.tuple.{Attribute, AttributeType}
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+class PythonUdfUiParameterInjectorSpec extends AnyFlatSpec with Matchers {
+
+ private def createParameter(
+ key: String,
+ attributeType: AttributeType,
+ value: String
+ ): UiUDFParameter = {
+ val parameter = new UiUDFParameter
+ parameter.attribute = new Attribute(key, attributeType)
+ parameter.value = value
+ parameter
+ }
+
+ private val baseUdfCode: String =
+ """from pytexera import *
+ |
+ |class ProcessTupleOperator(UDFOperatorV2):
+ | @overrides
+ | def open(self):
+ | print("open")
+ |
+ | @overrides
+ | def process_tuple(self, tuple_: Tuple, port: int):
+ | yield tuple_
+ |""".stripMargin
+
+ it should "return encoded user code unchanged when there are no ui parameters" in {
+ val injectedCode = PythonUdfUiParameterInjector.inject(baseUdfCode, Nil)
+
+ injectedCode should include("class ProcessTupleOperator(UDFOperatorV2):")
+ injectedCode should include("""print("open")""")
+ injectedCode should not include ("_texera_injected_ui_parameters")
+ injectedCode should not include ("self.decode_python_template")
+ }
+
+ it should "inject ui parameter hook into supported UDF class" in {
+ val injectedCode = PythonUdfUiParameterInjector.inject(
+ baseUdfCode,
+ List(
+ createParameter("date", AttributeType.TIMESTAMP, "2024-01-01T00:00:00Z")
+ )
+ )
+
+ injectedCode should include("class ProcessTupleOperator(UDFOperatorV2):")
+ injectedCode should include("def _texera_injected_ui_parameters(self):")
+ injectedCode should include("return {")
+ injectedCode should include("self.decode_python_template")
+ injectedCode should include("""print("open")""")
+ }
+
+ it should "inject the reserved hook before the first method definition" in {
+ val injectedCode = PythonUdfUiParameterInjector.inject(
+ baseUdfCode,
+ List(createParameter("k", AttributeType.STRING, "v"))
+ )
+
+ val hookIndex = injectedCode.indexOf("def _texera_injected_ui_parameters(self):")
+ val openIndex = injectedCode.indexOf("def open(self):")
+
+ hookIndex should be >= 0
+ openIndex should be > hookIndex
+ }
Review Comment:
The test has two issues:
1. `hookIndex = injectedCode.indexOf("def _texera_injected_ui_parameters(self):")` will always return `-1` because the injected method signature includes a return type annotation: `def _texera_injected_ui_parameters(self) -> typing.Dict[str, typing.Any]:`. The substring `(self):` is not present. The assertion `hookIndex should be >= 0` will fail.
2. Even if the string search were fixed, `injectHookIntoUserClass` appends the hook at `classBlockEnd` (end of class body), placing it *after* `def open(self):`. This means `hookIndex > openIndex`, so the assertion `openIndex should be > hookIndex` would also fail. The dead method `findInsertionPointInsideClass` (line 131-136) was apparently intended to solve this by inserting before the first method, but it is never called.
##########
amber/src/main/python/pytexera/udf/udf_operator.py:
##########
@@ -16,12 +16,76 @@
# under the License.
from abc import abstractmethod
-from typing import Iterator, Optional, Union
-
-from pyamber import *
+from typing import Any, Dict, Iterator, Optional, Union
+import functools
-class UDFOperatorV2(TupleOperatorV2):
+from pyamber import *
+from core.models.schema.attribute_type import AttributeType, FROM_STRING_PARSER_MAPPING
+
+class _UiParameterSupport:
+ _ui_parameter_injected_values: Dict[str, Any] = {}
+ _ui_parameter_name_types: Dict[str, AttributeType] = {}
+
+ # Reserved hook name. Backend injector will generate this in the user's class.
+ def _texera_injected_ui_parameters(self) -> Dict[str, Any]:
+ return {}
+
+ def _texera_apply_injected_ui_parameters(self) -> None:
+ values = self._texera_injected_ui_parameters()
+ # Write to base class storage (not cls) because UiParameter reads from _UiParameterSupport directly
+ _UiParameterSupport._ui_parameter_injected_values = dict(values or {})
+ _UiParameterSupport._ui_parameter_name_types = {}
+
+ def __init_subclass__(cls, **kwargs):
+ super().__init_subclass__(**kwargs)
+
+ # Wrap only methods defined on this class (not inherited ones)
+ original_open = getattr(cls, "open", None)
+ if original_open is None:
+ return
+
+ # Avoid double wrapping
+ if getattr(original_open, "__texera_ui_params_wrapped__", False):
+ return
+
+ @functools.wraps(original_open)
+ def wrapped_open(self, *args, **kwargs):
+ self._texera_apply_injected_ui_parameters()
+ return original_open(self, *args, **kwargs)
+
+ setattr(wrapped_open, "__texera_ui_params_wrapped__", True)
+ cls.open = wrapped_open
+
+ class UiParameter:
+ def __init__(self, name: str, type: AttributeType):
+ if not isinstance(type, AttributeType):
+ raise TypeError(
+ f"UiParameter.type must be an AttributeType, got {type!r}."
+ )
+
+ existing_type = _UiParameterSupport._ui_parameter_name_types.get(name)
+ if existing_type is not None and existing_type != type:
+ raise ValueError(
+ f"Duplicate UiParameter name '{name}' with conflicting types: "
+ f"{existing_type.name} vs {type.name}."
+ )
+
+ _UiParameterSupport._ui_parameter_name_types[name] = type
+ raw_value = _UiParameterSupport._ui_parameter_injected_values.get(name)
+ self.name = name
+ self.type = type
+ self.value = _UiParameterSupport._parse(raw_value, type)
Review Comment:
The `UiParameter.__init__` method uses `type` as a parameter name (line 61), which shadows the Python built-in `type()` function. While this works correctly inside the function, it is a Python anti-pattern that can lead to subtle bugs if developers modify this code and inadvertently try to use `type()`. A clearer name like `attr_type` would be more consistent with `_parse`'s parameter name `attr_type` used on line 81.
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/tuple/Attribute.java:
##########
@@ -21,6 +21,10 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.texera.amber.pybuilder.EncodableStringAnnotation;
+import org.apache.texera.amber.pybuilder.PyStringTypes;
+import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableStringFactory$;
Review Comment:
Two unused imports were added: `import org.apache.texera.amber.pybuilder.PyStringTypes;` (line 25) and `import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableStringFactory$;` (line 26). Neither `PyStringTypes` nor `EncodableStringFactory$` are referenced anywhere in the file body — only `@EncodableStringAnnotation` from line 24's import is used. These unused imports should be removed.
##########
amber/src/main/python/pytexera/udf/udf_operator.py:
##########
@@ -16,12 +16,76 @@
# under the License.
from abc import abstractmethod
-from typing import Iterator, Optional, Union
-
-from pyamber import *
+from typing import Any, Dict, Iterator, Optional, Union
+import functools
-class UDFOperatorV2(TupleOperatorV2):
+from pyamber import *
+from core.models.schema.attribute_type import AttributeType, FROM_STRING_PARSER_MAPPING
+
+class _UiParameterSupport:
+ _ui_parameter_injected_values: Dict[str, Any] = {}
+ _ui_parameter_name_types: Dict[str, AttributeType] = {}
+
+ # Reserved hook name. Backend injector will generate this in the user's class.
+ def _texera_injected_ui_parameters(self) -> Dict[str, Any]:
+ return {}
+
+ def _texera_apply_injected_ui_parameters(self) -> None:
+ values = self._texera_injected_ui_parameters()
+ # Write to base class storage (not cls) because UiParameter reads from _UiParameterSupport directly
+ _UiParameterSupport._ui_parameter_injected_values = dict(values or {})
+ _UiParameterSupport._ui_parameter_name_types = {}
+
+ def __init_subclass__(cls, **kwargs):
+ super().__init_subclass__(**kwargs)
+
+ # Wrap only methods defined on this class (not inherited ones)
+ original_open = getattr(cls, "open", None)
+ if original_open is None:
+ return
+
+ # Avoid double wrapping
+ if getattr(original_open, "__texera_ui_params_wrapped__", False):
+ return
+
+ @functools.wraps(original_open)
+ def wrapped_open(self, *args, **kwargs):
+ self._texera_apply_injected_ui_parameters()
+ return original_open(self, *args, **kwargs)
+
+ setattr(wrapped_open, "__texera_ui_params_wrapped__", True)
+ cls.open = wrapped_open
+
+ class UiParameter:
+ def __init__(self, name: str, type: AttributeType):
+ if not isinstance(type, AttributeType):
+ raise TypeError(
+ f"UiParameter.type must be an AttributeType, got {type!r}."
+ )
+
+ existing_type = _UiParameterSupport._ui_parameter_name_types.get(name)
+ if existing_type is not None and existing_type != type:
+ raise ValueError(
+ f"Duplicate UiParameter name '{name}' with conflicting types: "
+ f"{existing_type.name} vs {type.name}."
+ )
+
+ _UiParameterSupport._ui_parameter_name_types[name] = type
+ raw_value = _UiParameterSupport._ui_parameter_injected_values.get(name)
+ self.name = name
+ self.type = type
+ self.value = _UiParameterSupport._parse(raw_value, type)
+
+ @staticmethod
+ def _parse(value: Any, attr_type: AttributeType) -> Any:
+ if value is None:
+ return None
+
+ py_type = FROM_STRING_PARSER_MAPPING.get(attr_type)
+ return py_type(value)
+
Review Comment:
The new `_UiParameterSupport` class and `UiParameter` inner class in `udf_operator.py` have no corresponding Python unit tests. The surrounding codebase has thorough test coverage (e.g., `test_echo_operator.py`, `test_count_batch_operator.py`). Tests should be added to cover at minimum: parameter value injection via `_texera_apply_injected_ui_parameters`, duplicate name detection with conflicting types in `UiParameter`, the `_parse` method for all supported attribute types, and the `__init_subclass__` wrapping behavior.
##########
frontend/src/app/workspace/component/code-editor-dialog/code-editor.component.ts:
##########
@@ -102,6 +103,7 @@ export class CodeEditorComponent implements AfterViewInit, SafeStyle, OnDestroy
private isMultipleVariables: boolean = false;
public codeDebuggerComponent!: Type<any> | null;
public editorToPass!: MonacoEditor;
+ // private readonly pythonCodeChangeSubject = new Subject<string>();
Review Comment:
The commented-out field `// private readonly pythonCodeChangeSubject = new Subject<string>();` on line 106 appears to be leftover dead code from a previous implementation approach. It should be removed to keep the file clean.
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-sync.service.ts:
##########
@@ -0,0 +1,131 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Injectable } from "@angular/core";
+import { isEqual } from "lodash-es";
+import { ReplaySubject } from "rxjs";
+import { Subject } from "rxjs";
Review Comment:
The `Subject` import from `"rxjs"` on line 22 is unused. Only `ReplaySubject` is actually used in this file. This unused import should be removed.
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +77,17 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: int,
+ AttributeType.LONG: int,
+ AttributeType.DOUBLE: float,
+ AttributeType.BOOL: lambda v: str(v).strip().lower() in ("True", "true", "1", "yes"),
Review Comment:
The boolean parser in `FROM_STRING_PARSER_MAPPING` for `AttributeType.BOOL` includes `"True"` in the truthy set (line 85), but this is redundant since the lambda already calls `str(v).strip().lower()` — after `.lower()`, no string can equal `"True"` (capital T). The `"True"` entry in the tuple is dead code and the set effectively only checks for `"true"`, `"1"`, and `"yes"`.
##########
amber/src/main/python/pytexera/udf/udf_operator.py:
##########
@@ -16,12 +16,76 @@
# under the License.
from abc import abstractmethod
-from typing import Iterator, Optional, Union
-
-from pyamber import *
+from typing import Any, Dict, Iterator, Optional, Union
+import functools
-class UDFOperatorV2(TupleOperatorV2):
+from pyamber import *
+from core.models.schema.attribute_type import AttributeType, FROM_STRING_PARSER_MAPPING
+
+class _UiParameterSupport:
+ _ui_parameter_injected_values: Dict[str, Any] = {}
+ _ui_parameter_name_types: Dict[str, AttributeType] = {}
+
+ # Reserved hook name. Backend injector will generate this in the user's class.
+ def _texera_injected_ui_parameters(self) -> Dict[str, Any]:
+ return {}
+
+ def _texera_apply_injected_ui_parameters(self) -> None:
+ values = self._texera_injected_ui_parameters()
+ # Write to base class storage (not cls) because UiParameter reads from _UiParameterSupport directly
+ _UiParameterSupport._ui_parameter_injected_values = dict(values or {})
+ _UiParameterSupport._ui_parameter_name_types = {}
+
+ def __init_subclass__(cls, **kwargs):
+ super().__init_subclass__(**kwargs)
+
+ # Wrap only methods defined on this class (not inherited ones)
+ original_open = getattr(cls, "open", None)
+ if original_open is None:
+ return
+
+ # Avoid double wrapping
+ if getattr(original_open, "__texera_ui_params_wrapped__", False):
+ return
+
+ @functools.wraps(original_open)
+ def wrapped_open(self, *args, **kwargs):
+ self._texera_apply_injected_ui_parameters()
+ return original_open(self, *args, **kwargs)
+
+ setattr(wrapped_open, "__texera_ui_params_wrapped__", True)
+ cls.open = wrapped_open
+
+ class UiParameter:
+ def __init__(self, name: str, type: AttributeType):
+ if not isinstance(type, AttributeType):
+ raise TypeError(
+ f"UiParameter.type must be an AttributeType, got {type!r}."
+ )
+
+ existing_type = _UiParameterSupport._ui_parameter_name_types.get(name)
+ if existing_type is not None and existing_type != type:
+ raise ValueError(
+ f"Duplicate UiParameter name '{name}' with conflicting types: "
+ f"{existing_type.name} vs {type.name}."
+ )
+
+ _UiParameterSupport._ui_parameter_name_types[name] = type
+ raw_value = _UiParameterSupport._ui_parameter_injected_values.get(name)
+ self.name = name
+ self.type = type
+ self.value = _UiParameterSupport._parse(raw_value, type)
+
+ @staticmethod
+ def _parse(value: Any, attr_type: AttributeType) -> Any:
+ if value is None:
+ return None
+
+ py_type = FROM_STRING_PARSER_MAPPING.get(attr_type)
+ return py_type(value)
Review Comment:
The `_parse` method in `_UiParameterSupport` (lines 81-86) does not handle the case where `FROM_STRING_PARSER_MAPPING.get(attr_type)` returns `None` (i.e., when the `attr_type` is not in the mapping). If `py_type` is `None`, calling `py_type(value)` on line 86 will raise a `TypeError: 'NoneType' object is not callable`, giving an unhelpful error to the user.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104569147
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/DualInputPortsPythonUDFOpDescV2.scala:
##########
@@ -34,11 +34,25 @@ class DualInputPortsPythonUDFOpDescV2 extends LogicalOp {
required = true,
defaultValue =
"# Choose from the following templates:\n" +
+ "# \n" +
+ "# UiParameter notes:\n" +
+ "# - A UiParameter is a user-editable value exposed in the property panel and read from your Python code.\n" +
+ "# - Define UiParameter values in open() and then use them later in your UDF methods.\n" +
+ "# - Active UiParameter calls appear in the property panel; commented-out calls are ignored.\n" +
+ "# - Supported UiParameter types are STRING, INT/LONG, DOUBLE, BOOL, and TIMESTAMP.\n" +
"# \n" +
"# from pytexera import *\n" +
"# \n" +
"# class ProcessTupleOperator(UDFOperatorV2):\n" +
- "# \n" +
+ "# @overrides\n" +
Review Comment:
Got it, which one should I keep? also I think the current design makes it clear the UiParameter is available for all the 3 types of classes, I would like to keep it as it is, if you think removing them is better that is fine for me.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104566238
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/visualization/wordCloud/WordCloudOpDesc.scala:
##########
Review Comment:
Yes, actually I dont know why this is happening, I run scalafmtAll and scalafixAll, any hints or help is appreciated.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105616636
##########
amber/src/main/python/pytexera/__init__.py:
##########
@@ -53,4 +54,7 @@
"Iterator",
"Optional",
"Union",
+ "Dict",
Review Comment:
@kunwp1 NameError: name 'Dict' is not defined
2026-04-18 12:17:21.586 | ERROR | core.architecture.rpc.async_rpc_server:receive:102 - name 'Dict' is not defined
Traceback (most recent call last):
File "C:\Users\carlo\AppData\Local\Programs\Python\Python312\Lib\threading.py", line 1032, in _bootstrap
self._bootstrap_inner()
| -> <function Thread._bootstrap_inner at 0x0000016D4CABF600>
-> <Thread(main_loop_thread, started 32596)>
File "C:\Users\carlo\AppData\Local\Programs\Python\Python312\Lib\threading.py", line 1075, in _bootstrap_inner
self.run()
| -> <function Thread.run at 0x0000016D4CABF2E0>
-> <Thread(main_loop_thread, started 32596)>
File "C:\Users\carlo\AppData\Local\Programs\Python\Python312\Lib\threading.py", line 1012, in run
self._target(*self._args, **self._kwargs)
| | | | | -> {}
| | | | -> <Thread(main_loop_thread, started 32596)>
| | | -> ()
| | -> <Thread(main_loop_thread, started 32596)>
| -> <bound method StoppableQueueBlockingRunnable.run of <core.runnables.main_loop.MainLoop object at 0x0000016D03002270>>
-> <Thread(main_loop_thread, started 32596)>
File "C:\Users\carlo\texera\texera\amber\src\main\python\core\util\stoppable\stoppable_queue_blocking_thread.py", line 65, in run
self.receive(self.interruptible_get())
| | | -> <function StoppableQueueBlockingRunnable.interruptible_get at 0x0000016D7F8FACA0>
| | -> <core.runnables.main_loop.MainLoop object at 0x0000016D03002270>
| -> <function MainLoop.receive at 0x0000016D02F84C20>
-> <core.runnables.main_loop.MainLoop object at 0x0000016D03002270>
File "C:\Users\carlo\texera\texera\amber\src\main\python\core\runnables\main_loop.py", line 142, in receive
match(
-> <function match at 0x0000016D001AE480>
File "C:\Users\carlo\texera\texera\.venv312\Lib\site-packages\pampy\pampy.py", line 299, in match
return run(action, lambda_args)
| | -> [DCMElement(tag=ChannelIdentity(from_worker_id=ActorVirtualIdentity(name='CONTROLLER'), to_worker_id=ActorVirtualIdentity(nam...
| -> <bound method MainLoop._process_dcm of <core.runnables.main_loop.MainLoop object at 0x0000016D03002270>>
-> <function run at 0x0000016D001AD580>
File "C:\Users\carlo\texera\texera\.venv312\Lib\site-packages\pampy\pampy.py", line 48, in run
return action(*var)
| -> [DCMElement(tag=ChannelIdentity(from_worker_id=ActorVirtualIdentity(name='CONTROLLER'), to_worker_id=ActorVirtualIdentity(nam...
-> <bound method MainLoop._process_dcm of <core.runnables.main_loop.MainLoop object at 0x0000016D03002270>>
File "C:\Users\carlo\texera\texera\amber\src\main\python\core\runnables\main_loop.py", line 224, in _process_dcm
match(
-> <function match at 0x0000016D001AE480>
File "C:\Users\carlo\texera\texera\.venv312\Lib\site-packages\pampy\pampy.py", line 299, in match
return run(action, lambda_args)
| | -> [ChannelIdentity(from_worker_id=ActorVirtualIdentity(name='CONTROLLER'), to_worker_id=ActorVirtualIdentity(name='Worker:WF78-...
| -> <bound method AsyncRPCServer.receive of <core.architecture.rpc.async_rpc_server.AsyncRPCServer object at 0x0000016D03002690>>
-> <function run at 0x0000016D001AD580>
File "C:\Users\carlo\texera\texera\.venv312\Lib\site-packages\pampy\pampy.py", line 48, in run
return action(*var)
| -> [ChannelIdentity(from_worker_id=ActorVirtualIdentity(name='CONTROLLER'), to_worker_id=ActorVirtualIdentity(name='Worker:WF78-...
-> <bound method AsyncRPCServer.receive of <core.architecture.rpc.async_rpc_server.AsyncRPCServer object at 0x0000016D03002690>>
> File "C:\Users\carlo\texera\texera\amber\src\main\python\core\architecture\rpc\async_rpc_server.py", line 94, in receive
asyncio.run(handler.func(control_payload_stream))
| | | | -> <core.architecture.rpc.async_rpc_server.AsyncRPCServer._wrap_as_stream.<locals>.ControlRequestStream object at 0x0000016D0300...
| | | -> _tuplegetter(0, 'Alias for field number 0')
| | -> Handler(func=<bound method WorkerServiceBase.__rpc_initialize_executor of <core.architecture.rpc.async_rpc_handler_initialize...
| -> <function run at 0x0000016D4CD31DA0>
-> <module 'asyncio' from 'C:\\Users\\carlo\\AppData\\Local\\Programs\\Python\\Python312\\Lib\\asyncio\\__init__.py'>
File "C:\Users\carlo\AppData\Local\Programs\Python\Python312\Lib\asyncio\runners.py", line 195, in run
return runner.run(main)
| | -> <coroutine object WorkerServiceBase.__rpc_initialize_executor at 0x0000016D02FB1D20>
| -> <function Runner.run at 0x0000016D4CFAB240>
-> <asyncio.runners.Runner object at 0x0000016D030014C0>
File "C:\Users\carlo\AppData\Local\Programs\Python\Python312\Lib\asyncio\runners.py", line 118, in run
return self._loop.run_until_complete(task)
| | -> <Task finished name='Task-1' coro=<WorkerServiceBase.__rpc_initialize_executor() done, defined at C:\Users\carlo\texera\texer...
| -> None
-> <asyncio.runners.Runner object at 0x0000016D030014C0>
File "C:\Users\carlo\AppData\Local\Programs\Python\Python312\Lib\asyncio\base_events.py", line 691, in run_until_complete
return future.result()
| -> <method 'result' of '_asyncio.Task' objects>
-> <Task finished name='Task-1' coro=<WorkerServiceBase.__rpc_initialize_executor() done, defined at C:\Users\carlo\texera\texer...
File "C:\Users\carlo\texera\texera\amber\src\main\python\proto\org\apache\texera\amber\engine\architecture\rpc\__init__.py", line 1597, in __rpc_initialize_executor
response = await self.initialize_executor(request)
| | -> InitializeExecutorRequest(total_worker_count=1, op_exec_init_info=OpExecInitInfo(op_exec_with_code=OpExecWithCode(code='# Cho...
| -> <function InitializeExecutorHandler.initialize_executor at 0x0000016D02F720C0>
-> <core.architecture.rpc.async_rpc_handler_initializer.AsyncRPCHandlerInitializer object at 0x0000016D02EFB320>
File "C:\Users\carlo\texera\texera\amber\src\main\python\core\architecture\handlers\control\initialize_executor_handler.py", line 30, in initialize_executor
self.context.executor_manager.initialize_executor(
| | | -> <function ExecutorManager.initialize_executor at 0x0000016D02F51440>
| | -> <core.architecture.managers.executor_manager.ExecutorManager object at 0x0000016D030022D0>
| -> <core.architecture.managers.context.Context object at 0x0000016D030022A0>
-> <core.architecture.rpc.async_rpc_handler_initializer.AsyncRPCHandlerInitializer object at 0x0000016D02EFB320>
File "C:\Users\carlo\texera\texera\amber\src\main\python\core\architecture\managers\executor_manager.py", line 151, in initialize_executor
executor: type(Operator) = self.load_executor_definition(code)
| | | -> '# Choose from the following templates:\n# \n# UiParameter notes:\n# - A UiParameter is a user-editable value exposed in the ...
| | -> <function ExecutorManager.load_executor_definition at 0x0000016D02F51260>
| -> <core.architecture.managers.executor_manager.ExecutorManager object at 0x0000016D030022D0>
-> <class 'core.models.operator.Operator'>
File "C:\Users\carlo\texera\texera\amber\src\main\python\core\architecture\managers\executor_manager.py", line 93, in load_executor_definition
executor_module = importlib.import_module(module_name)
| | -> 'udf-v1'
| -> <function import_module at 0x0000016D4CB6D080>
-> <module 'importlib' from 'C:\\Users\\carlo\\AppData\\Local\\Programs\\Python\\Python312\\Lib\\importlib\\__init__.py'>
File "C:\Users\carlo\AppData\Local\Programs\Python\Python312\Lib\importlib\__init__.py", line 90, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
| | | | | -> 0
| | | | -> None
| | | -> 0
| | -> 'udf-v1'
| -> <function _gcd_import at 0x0000016D4C48C0E0>
-> <module '_frozen_importlib' (frozen)>
File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
File "<frozen importlib._bootstrap_external>", line 999, in exec_module
File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
File "C:\Users\carlo\AppData\Local\Temp\tmpkqz8h60kfsTempFS\udf-v1.py", line 12, in <module>
class ProcessTupleOperator(UDFOperatorV2):
-> <class 'pytexera.udf.udf_operator.UDFOperatorV2'>
File "C:\Users\carlo\AppData\Local\Temp\tmpkqz8h60kfsTempFS\udf-v1.py", line 30, in ProcessTupleOperator
def _texera_injected_ui_parameters(self) -> Dict[str, Any]:
NameError: name 'Dict' is not defined
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105566375
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/DualInputPortsPythonUDFOpDescV2.scala:
##########
@@ -34,11 +34,25 @@ class DualInputPortsPythonUDFOpDescV2 extends LogicalOp {
required = true,
defaultValue =
"# Choose from the following templates:\n" +
+ "# \n" +
+ "# UiParameter notes:\n" +
+ "# - A UiParameter is a user-editable value exposed in the property panel and read from your Python code.\n" +
+ "# - Define UiParameter values in open() and then use them later in your UDF methods.\n" +
+ "# - Active UiParameter calls appear in the property panel; commented-out calls are ignored.\n" +
+ "# - Supported UiParameter types are STRING, INT/LONG, DOUBLE, BOOL, and TIMESTAMP.\n" +
"# \n" +
"# from pytexera import *\n" +
"# \n" +
"# class ProcessTupleOperator(UDFOperatorV2):\n" +
- "# \n" +
+ "# @overrides\n" +
Review Comment:
Ok
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,435 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import {
+ AttributeType,
+ JavaAttributeTypeName,
+ PythonAttributeTypeName,
+ SchemaAttribute,
+ JAVA_ATTRIBUTE_TYPE_NAMES,
+ PYTHON_ATTRIBUTE_TYPE_NAMES,
+} from "../../types/workflow-compiling.interface";
+
+export interface UiUdfParameter {
+ attribute: SchemaAttribute;
+ value: string;
+}
+
+type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName;
+
+const ATTRIBUTE_TYPE_TOKEN_TO_CANONICAL: Readonly<Record<ParserAttributeTypeToken, AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+ BINARY: "binary",
+ LARGE_BINARY: "large_binary",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "string",
+ "integer",
+ "long",
+ "double",
+ "boolean",
+ "timestamp",
+]);
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersParserService {
+ private static readonly SUPPORTED_CLASSES = [
+ "ProcessTupleOperator",
+ "ProcessBatchOperator",
+ "ProcessTableOperator",
+ "GenerateOperator",
+ ];
+
+ parse(code: string): UiUdfParameter[] {
+ if (!code) {
+ return [];
+ }
+
+ const sanitizedCode = this.stripCommentsAndDocstrings(code);
+ const classPattern = UiUdfParametersParserService.SUPPORTED_CLASSES.join("|");
+ const classRegex = new RegExp(
+ `class\\s+(${classPattern})\\s*\\([^)]*\\)\\s*:[\\s\\S]*?(?=\\nclass\\s+\\w+\\s*\\(|$)`,
+ "g"
+ );
+
+ const parsed: UiUdfParameter[] = [];
+ const existingNames = new Set<string>();
+
+ let classMatch: RegExpExecArray | null;
+ while ((classMatch = classRegex.exec(sanitizedCode)) !== null) {
+ const classBlock = classMatch[0];
+
+ for (const args of this.extractUiParameterArgumentLists(classBlock)) {
+ const argumentTokens = this.tokenizeArguments(args);
+ const extracted = this.extractParameter(argumentTokens);
+ if (!extracted || existingNames.has(extracted.attribute.attributeName)) {
+ continue;
+ }
+
+ existingNames.add(extracted.attribute.attributeName);
+ parsed.push(extracted);
+ }
+ }
+
+ return parsed;
+ }
+
+ private stripCommentsAndDocstrings(code: string): string {
Review Comment:
Ok
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105565081
##########
amber/src/main/python/core/models/schema/attribute_type.py:
##########
@@ -78,6 +78,45 @@ class AttributeType(Enum):
}
+FROM_STRING_PARSER_MAPPING = {
+ AttributeType.STRING: str,
+ AttributeType.INT: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.LONG: lambda v: (
+ 0 if v is None or (isinstance(v, str) and v.strip() == "") else int(v)
+ ),
+ AttributeType.DOUBLE: lambda v: (
+ 0.0 if v is None or (isinstance(v, str) and v.strip() == "") else float(v)
+ ),
+ AttributeType.BOOL: lambda v: (
+ False
+ if v is None or (isinstance(v, str) and v.strip() == "")
+ else str(v).strip().lower() in ("true", "1", "yes")
Review Comment:
1) I agree a dropdown is better, but for the first version and given this PR is so delayed, I suggest we leave it for V2.
2) Ok, can you give more context of why checking the text input scan?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105616636
##########
amber/src/main/python/pytexera/__init__.py:
##########
@@ -53,4 +54,7 @@
"Iterator",
"Optional",
"Union",
+ "Dict",
Review Comment:
@kunwp1 NameError: name 'Dict' is not defined
2026-04-18 12:17:21.586 | ERROR | core.architecture.rpc.async_rpc_server:receive:102 - name 'Dict' is not defined
File "C:\Users\carlo\AppData\Local\Temp\tmpkqz8h60kfsTempFS\udf-v1.py", line 12, in <module>
class ProcessTupleOperator(UDFOperatorV2):
-> <class 'pytexera.udf.udf_operator.UDFOperatorV2'>
File "C:\Users\carlo\AppData\Local\Temp\tmpkqz8h60kfsTempFS\udf-v1.py", line 30, in ProcessTupleOperator
def _texera_injected_ui_parameters(self) -> Dict[str, Any]:
NameError: name 'Dict' is not defined
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3035949792
##########
frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.css:
##########
@@ -16,4 +16,3 @@
* specific language governing permissions and limitations
* under the License.
*/
-
Review Comment:
Ok
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/source/PythonUDFSourceOpDescV2.scala:
##########
@@ -27,18 +27,20 @@ import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, Workflow
import org.apache.texera.amber.core.workflow.{OutputPort, PhysicalOp, SchemaPropagationFunc}
import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorInfo}
import org.apache.texera.amber.operator.source.SourceOperatorDescriptor
+import org.apache.texera.amber.operator.udf.python.{PythonUdfUiParameterInjector, UiUDFParameter}
class PythonUDFSourceOpDescV2 extends SourceOperatorDescriptor {
@JsonProperty(
required = true,
- defaultValue = "# from pytexera import *\n" +
- "# class GenerateOperator(UDFSourceOperator):\n" +
- "# \n" +
- "# @overrides\n" +
- "# \n" +
- "# def produce(self) -> Iterator[Union[TupleLike, TableLike, None]]:\n" +
- "# yield\n"
+ defaultValue =
Review Comment:
Ok
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4302124437
For our records, please also include the reason of this decision.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4309008982
I feel the code can be reused as multiple PRs. So the effort is not wasted.
Quality control is important for the project. We don't have a fixed number of lines of code. I hope other people can chime in. @kunwp1 @Yicong-Huang @aglinxinyuan ?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104574425
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
Review Comment:
The reason is to unify them since there are mismatchs between python namings and java namings, and it may cause issues in the backend.
##########
frontend/src/app/workspace/service/code-editor/ui-udf-parameters-parser.service.ts:
##########
@@ -0,0 +1,435 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import {
+ AttributeType,
+ JavaAttributeTypeName,
+ PythonAttributeTypeName,
+ SchemaAttribute,
+ JAVA_ATTRIBUTE_TYPE_NAMES,
+ PYTHON_ATTRIBUTE_TYPE_NAMES,
+} from "../../types/workflow-compiling.interface";
+
+export interface UiUdfParameter {
+ attribute: SchemaAttribute;
+ value: string;
+}
+
+type ParserAttributeTypeToken = JavaAttributeTypeName | PythonAttributeTypeName;
+
+const ATTRIBUTE_TYPE_TOKEN_TO_CANONICAL: Readonly<Record<ParserAttributeTypeToken, AttributeType>> = {
+ STRING: "string",
+ INTEGER: "integer",
+ INT: "integer",
+ LONG: "long",
+ DOUBLE: "double",
+ BOOLEAN: "boolean",
+ BOOL: "boolean",
+ TIMESTAMP: "timestamp",
+ BINARY: "binary",
+ LARGE_BINARY: "large_binary",
+};
+
+const JAVA_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(JAVA_ATTRIBUTE_TYPE_NAMES);
+const PYTHON_ATTRIBUTE_TYPE_NAME_SET = new Set<string>(PYTHON_ATTRIBUTE_TYPE_NAMES);
+const SUPPORTED_UI_PARAMETER_ATTRIBUTE_TYPES = new Set<AttributeType>([
+ "string",
+ "integer",
+ "long",
+ "double",
+ "boolean",
+ "timestamp",
+]);
+
+@Injectable({ providedIn: "root" })
+export class UiUdfParametersParserService {
+ private static readonly SUPPORTED_CLASSES = [
+ "ProcessTupleOperator",
+ "ProcessBatchOperator",
+ "ProcessTableOperator",
+ "GenerateOperator",
+ ];
+
+ parse(code: string): UiUdfParameter[] {
+ if (!code) {
+ return [];
+ }
+
+ const sanitizedCode = this.stripCommentsAndDocstrings(code);
+ const classPattern = UiUdfParametersParserService.SUPPORTED_CLASSES.join("|");
+ const classRegex = new RegExp(
+ `class\\s+(${classPattern})\\s*\\([^)]*\\)\\s*:[\\s\\S]*?(?=\\nclass\\s+\\w+\\s*\\(|$)`,
+ "g"
+ );
+
+ const parsed: UiUdfParameter[] = [];
+ const existingNames = new Set<string>();
+
+ let classMatch: RegExpExecArray | null;
+ while ((classMatch = classRegex.exec(sanitizedCode)) !== null) {
+ const classBlock = classMatch[0];
+
+ for (const args of this.extractUiParameterArgumentLists(classBlock)) {
+ const argumentTokens = this.tokenizeArguments(args);
+ const extracted = this.extractParameter(argumentTokens);
+ if (!extracted || existingNames.has(extracted.attribute.attributeName)) {
+ continue;
+ }
+
+ existingNames.add(extracted.attribute.attributeName);
+ parsed.push(extracted);
+ }
+ }
+
+ return parsed;
+ }
+
+ private stripCommentsAndDocstrings(code: string): string {
Review Comment:
Yes, I did not check, do you have something in mind meanwhile?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3104566820
##########
frontend/src/app/common/formly/collab-wrapper/collab-wrapper/collab-wrapper.component.css:
##########
Review Comment:
Yes, actually I dont know why this is happening, I run scalafmtAll and scalafixAll, any hints or help is appreciated.
Thanks.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3025295031
##########
common/workflow-core/src/main/scala/org/apache/texera/amber/core/tuple/Attribute.java:
##########
@@ -21,6 +21,10 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
+import org.apache.texera.amber.pybuilder.EncodableStringAnnotation;
+import org.apache.texera.amber.pybuilder.PyStringTypes;
+import org.apache.texera.amber.pybuilder.PyStringTypes.EncodableStringFactory$;
Review Comment:
Hahaha that is crazy
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3069145166
##########
frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.css:
##########
@@ -16,4 +16,3 @@
* specific language governing permissions and limitations
* under the License.
*/
-
Review Comment:
Fixed
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3034663418
##########
common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/udf/python/source/PythonUDFSourceOpDescV2.scala:
##########
@@ -27,18 +27,20 @@ import org.apache.texera.amber.core.virtualidentity.{ExecutionIdentity, Workflow
import org.apache.texera.amber.core.workflow.{OutputPort, PhysicalOp, SchemaPropagationFunc}
import org.apache.texera.amber.operator.metadata.{OperatorGroupConstants, OperatorInfo}
import org.apache.texera.amber.operator.source.SourceOperatorDescriptor
+import org.apache.texera.amber.operator.udf.python.{PythonUdfUiParameterInjector, UiUDFParameter}
class PythonUDFSourceOpDescV2 extends SourceOperatorDescriptor {
@JsonProperty(
required = true,
- defaultValue = "# from pytexera import *\n" +
- "# class GenerateOperator(UDFSourceOperator):\n" +
- "# \n" +
- "# @overrides\n" +
- "# \n" +
- "# def produce(self) -> Iterator[Union[TupleLike, TableLike, None]]:\n" +
- "# yield\n"
+ defaultValue =
Review Comment:
avoid unnecessary change.
##########
frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.css:
##########
@@ -16,4 +16,3 @@
* specific language governing permissions and limitations
* under the License.
*/
-
Review Comment:
avoid unnecessary change.
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
+ "STRING",
+ "INTEGER",
+ "LONG",
+ "DOUBLE",
+ "BOOLEAN",
+ "TIMESTAMP",
+ "BINARY",
+ "LARGE_BINARY",
+] as const;
+
+export type JavaAttributeTypeName = (typeof JAVA_ATTRIBUTE_TYPE_NAMES)[number];
+
+// Python enum constant names (core.models.AttributeType)
+export const PYTHON_ATTRIBUTE_TYPE_NAMES = [
+ "STRING",
+ "INT",
+ "LONG",
+ "DOUBLE",
+ "BOOL",
+ "TIMESTAMP",
+ "BINARY",
+ "LARGE_BINARY",
+] as const;
+
+export type PythonAttributeTypeName = (typeof PYTHON_ATTRIBUTE_TYPE_NAMES)[number];
+
+// Useful when parsing code from either side
+export type AttributeTypeToken = AttributeType | JavaAttributeTypeName | PythonAttributeTypeName;
Review Comment:
Why do we need to define types manually in each language? Shouldn't they be converted automatically, or consider using the protobuf class?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4249876279
> @chenlica I will fix it after the current code pass the review, that is more efficient I think.
I just did the fix since it's caused by my changes. It's usually easier to resolve conflicts earlier than later.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #4268:
URL: https://github.com/apache/texera/pull/4268#discussion_r3105746677
##########
frontend/src/app/workspace/types/workflow-compiling.interface.ts:
##########
@@ -70,8 +70,50 @@ export type CompilationStateInfo = Readonly<
operatorErrors: Readonly<Record<string, WorkflowFatalError>>;
}
>;
+
// possible types of an attribute
-export type AttributeType = "string" | "integer" | "double" | "boolean" | "long" | "timestamp" | "binary"; // schema: an array of attribute names and types
+// Canonical frontend / JSON schema names
+export type AttributeType =
+ | "string"
+ | "integer"
+ | "long"
+ | "double"
+ | "boolean"
+ | "timestamp"
+ | "binary"
+ | "large_binary";
+
+// Java enum constant names (AttributeType.java)
+export const JAVA_ATTRIBUTE_TYPE_NAMES = [
Review Comment:
Done
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 closed pull request #4268: feat(frontend): Support for UDF Ui Parameter
URL: https://github.com/apache/texera/pull/4268
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4311542136
I don't have a specific limit for LoC, but 1,500+ lines across 11 new files is a lot to review at once, especially since it changes both the frontend and backend. Reviewing new files is naturally harder because we lack existing context. To make this smoother, I'd suggest making the PR description more robust, including an architecture diagram, a breakdown of new modules, and an explanation of the new APIs. Also, for features of this scale, it might be helpful to have a quick walkthrough with reviewers before the formal request. This gives everyone the necessary context to review efficiently.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(frontend): Support for UDF Ui Parameter [texera]
Posted by "kunwp1 (via GitHub)" <gi...@apache.org>.
kunwp1 commented on PR #4268:
URL: https://github.com/apache/texera/pull/4268#issuecomment-4302025352
We had an offline discussion and decided to split this PR.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org