voidmatcha commented on code in PR #5265:
URL: https://github.com/apache/zeppelin/pull/5265#discussion_r3367027821
##########
zeppelin-web-angular/src/app/interfaces/interpreter.ts:
##########
@@ -65,13 +65,48 @@ interface SnapshotPolicy {
interface Properties {
[key: string]: {
name: string;
- value: boolean;
- type: string;
+ value: string | number | boolean | null;
+ type: InterpreterPropertyTypes;
defaultValue?: string;
Review Comment:
```suggestion
defaultValue?: string | boolean;
```
nit: `defaultValue` arrives as a real JSON boolean for checkbox properties.
The setting templates use unquoted booleans
(jdbc/src/main/resources/interpreter-setting.json:115 `"defaultValue": false`)
##########
zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.ts:
##########
@@ -86,31 +122,38 @@ export class InterpreterItemComponent extends
DestroyHookComponent implements On
this.addProperties();
this.addDependence();
const formData = this.formGroup.getRawValue();
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- const properties: Record<any, any> = {};
-
- formData.properties
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- .sort((e: any) => e.key)
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- .forEach((e: any) => {
- const { key, value, type } = e;
- properties[key] = {
- value,
- type,
- name: key
- };
- });
- formData.properties = properties;
- // eslint-disable-next-line @typescript-eslint/no-explicit-any
- formData.dependencies.forEach((e: any) => {
- e.exclusions = e.exclusions.split(',').filter((s: string) => s !== '');
+ const properties: Record<string, InterpreterPropertyValue> = {};
+
+ formData.properties.forEach(({ key, value, type }) => {
+ properties[key] = {
+ // Numeric inputs hold JS numbers, which Gson would deserialize as
Double
+ // (e.g. 60000 -> "60000.0") and break Long/Integer parsing
(ZEPPELIN-6395),
+ // so send them as strings. Checkboxes stay real booleans, as in the
classic UI.
+ value: type === 'checkbox' ? value === true || value === 'true' :
String(value ?? ''),
+ type,
+ name: key
+ };
});
+ // session/process are UI-only state derived from perNote/perUser;
+ // the server-side InterpreterOption has no such fields, so they are not
sent.
+ const { isExistingProcess, isUserImpersonate, owners, perNote, perUser,
port, host, remote, setPermission } =
+ formData.option;
+ const setting: InterpreterSettingRequest = {
+ name: formData.name,
+ group: formData.group,
+ option: { isExistingProcess, isUserImpersonate, owners, perNote,
perUser, port, host, remote, setPermission },
+ properties,
+ dependencies: formData.dependencies.map(({ groupArtifactVersion,
exclusions }) => ({
+ groupArtifactVersion,
+ exclusions: exclusions.split(',').filter(s => s !== '')
Review Comment:
```suggestion
exclusions: exclusions
.split(',')
.map(s => s.trim())
.filter(s => s !== '')
```
nit: no per-entry `.trim()`, so `"org.foo:bar, org.baz:qux"` stores `"
org.baz:qux"` with a leading space, which won't match as a Maven exclusion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
