Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix operators eslint issues #4388

Merged
merged 6 commits into from
May 20, 2024
Merged

Fix operators eslint issues #4388

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
735ae3d
add operator specific eslint
ritch May 10, 2024
fd50a4e
fix some js operator module eslint issues
ritch May 10, 2024
3a35e02
undo exhaustive-deps fix
ritch May 10, 2024
bd4af84
more lint fixes for operators
ritch May 12, 2024
c395c57
rm 2204 install
benjaminpkane May 20, 2024
e365237
Merge branch 'develop' into operators-eslint
benjaminpkane May 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
more lint fixes for operators
  • Loading branch information
@ritch
ritch committed May 12, 2024
commit bd4af84fcc7101f5449dda98eac0f8ed6866e562
2 changes: 1 addition & 1 deletion app/packages/operators/src/OperatorPalette.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export default function OperatorPalette(props: OperatorPaletteProps) {
);
}

type SubmitButtonOption = {
export type SubmitButtonOption = {
id: string;
label: string;
};
Expand Down
16 changes: 8 additions & 8 deletions app/packages/operators/src/built-in-operators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ class OpenPanel extends Operator {
const openedPanels = useSpaceNodes(FIFTYONE_SPACE_ID);
return { availablePanels, openedPanels, spaces };
}
findFirstPanelContainer(node: SpaceNode): SpaceNode {
findFirstPanelContainer(node: SpaceNode): SpaceNode | null {
if (node.isPanelContainer()) {
return node;
}
Expand Down Expand Up @@ -461,7 +461,7 @@ class SetView extends Operator {
unlisted: true,
});
}
useHooks(): {} {
useHooks(): object {
const refetchableSavedViews = useRefetchableSavedViews();

return {
Expand Down Expand Up @@ -521,7 +521,7 @@ class ShowSamples extends Operator {
});
return new types.Property(inputs);
}
useHooks(): {} {
useHooks(): object {
return {
setView: fos.useSetView(),
};
Expand Down Expand Up @@ -596,7 +596,7 @@ class ShowOutput extends Operator {
unlisted: true,
});
}
async resolveInput(ctx: ExecutionContext): Promise<types.Property> {
async resolveInput(): Promise<types.Property> {
const inputs = new types.Object();
inputs.defineProperty("outputs", new types.Object(), {
label: "Outputs",
Expand All @@ -608,7 +608,7 @@ class ShowOutput extends Operator {
});
return new types.Property(inputs);
}
useHooks(ctx: ExecutionContext): {} {
useHooks(): object {
return {
io: useShowOperatorIO(),
};
Expand All @@ -631,7 +631,7 @@ class SetProgress extends Operator {
unlisted: true,
});
}
async resolveInput(ctx: ExecutionContext): Promise<types.Property> {
async resolveInput(): Promise<types.Property> {
const inputs = new types.Object();
inputs.defineProperty("label", new types.String(), { label: "Label" });
inputs.defineProperty("variant", new types.Enum(["linear", "circular"]), {
Expand All @@ -642,7 +642,7 @@ class SetProgress extends Operator {
});
return new types.Property(inputs);
}
useHooks(ctx: ExecutionContext): {} {
useHooks(): object {
return {
io: useShowOperatorIO(),
};
Expand Down Expand Up @@ -730,7 +730,7 @@ class SetSelectedLabels extends Operator {
unlisted: true,
});
}
useHooks(ctx: ExecutionContext): {} {
useHooks(): object {
return {
setSelected: fos.useSetSelectedLabels(),
};
Expand Down
3 changes: 2 additions & 1 deletion app/packages/operators/src/components/OperatorPromptFooter.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { useCallback } from "react";
import SplitButton from "../SplitButton";
import { BaseStylesProvider } from "../styled-components";
import { onEnter } from "../utils";
import { SubmitButtonOption } from "../OperatorPalette";

export default function OperatorPromptFooter(props: OperatorFooterProps) {
const {
Expand Down Expand Up @@ -91,7 +92,7 @@ type OperatorFooterProps = {
disableSubmit?: boolean;
disabledReason?: string;
loading?: boolean;
submitButtonOptions: any[];
submitButtonOptions: SubmitButtonOption[];
hasSubmitButtonOptions: boolean;
submitOptionsLoading: boolean;
showWarning?: boolean;
Expand Down
60 changes: 42 additions & 18 deletions app/packages/operators/src/operators.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,20 @@ import { stringifyError } from "./utils";
import { ValidationContext, ValidationError } from "./validation";
import { ExecutionCallback, OperatorExecutorOptions } from "./types-internal";

type RawInvocationRequest = {
operator_uri?: string;
operator_name?: string;
params: object;
options: object;
};

class InvocationRequest {
constructor(
public operatorURI: string,
public params: unknown = {},
public options?: OperatorExecutorOptions
) {}
static fromJSON(json: any) {
static fromJSON(json: RawInvocationRequest): InvocationRequest {
return new InvocationRequest(
json.operator_uri || json.operator_name,
json.params,
Expand Down Expand Up @@ -42,13 +49,13 @@ export class Executor {
queue.add(request);
}
}
static fromJSON(json: any) {
static fromJSON(json: { requests: RawInvocationRequest[]; logs: string[] }) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tip

Codebase Verification

The Executor.fromJSON method lacks robust error handling mechanisms for malformed JSON inputs. It directly processes the JSON data without validation, which could lead to runtime errors if the JSON structure is incorrect or improperly formatted. This confirms the concern raised in the review comment.

  • Location: app/packages/operators/src/operators.ts (Lines 52-57)
Analysis chain

The fromJSON method in the Executor class correctly maps JSON input to class instances. Ensure that error handling is robust, especially for malformed JSON inputs.

Scripts Executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify error handling for malformed JSON in `Executor.fromJSON`.

# Test: Search for error handling logic in `Executor.fromJSON`. Expect: Robust error handling.
rg --type ts 'Executor.fromJSON' app/packages/operators/src/operators.ts

Length of output: 116

Script:

#!/bin/bash
# Description: Search for the definition of `Executor.fromJSON` to inspect error handling logic.

# Using ast-grep to find the method definition and its contents.
ast-grep --lang typescript --pattern $'class Executor {
  $$$
  fromJSON($_) {
    $$$
  }
  $$$  
}'

Length of output: 2243

return new Executor(
json.requests.map((r: any) => InvocationRequest.fromJSON(r)),
json.logs
);
}
trigger(operatorURI: string, params: any = {}) {
trigger(operatorURI: string, params: object = {}) {
operatorURI = resolveOperatorURI(operatorURI);
this.requests.push(new InvocationRequest(operatorURI, params));
}
Expand All @@ -57,19 +64,32 @@ export class Executor {
}
}

export type RawContext = {
datasetName: string;
extended: boolean;
view: string;
filters: object;
selectedSamples: Set<string>;
selectedLabels: any[];
currentSample: string;
viewName: string;
delegationTarget: string;
requestDelegation: boolean;
state: CallbackInterface;
};
export class ExecutionContext {
public state: CallbackInterface;
constructor(
public params: any = {},
public _currentContext: any,
public hooks: any = {},
public params: object = {},
public _currentContext: RawContext,
public hooks: object = {},
public executor: Executor = null
) {
this.state = _currentContext.state;
}
public delegationTarget: string = null;
public requestDelegation: boolean = false;
trigger(operatorURI: string, params: any = {}) {
public requestDelegation = false;
trigger(operatorURI: string, params: object = {}) {
if (!this.executor) {
throw new Error(
"Cannot trigger operator from outside of an execution context"
Expand All @@ -93,9 +113,9 @@ function isObjWithContent(obj: any) {
export class OperatorResult {
constructor(
public operator: Operator,
public result: any = {},
public result: object = {},
public executor: Executor = null,
public error: any,
public error: string,
public delegated: boolean = false
) {}
hasOutputContent() {
Expand Down Expand Up @@ -227,7 +247,7 @@ export class Operator {
}
return false;
}
useHooks(ctx: ExecutionContext) {
useHooks(): object {
// This can be overridden to use hooks in the execute function
return {};
}
Expand All @@ -243,19 +263,23 @@ export class Operator {
}
return null;
}
async resolvePlacement(
ctx: ExecutionContext
): Promise<void | types.Placement> {}
async execute(ctx: ExecutionContext) {
async resolvePlacement(): Promise<void | types.Placement> {
return null;
}
async execute() {
throw new Error(`Operator ${this.uri} does not implement execute`);
}
public isRemote: boolean = false;
static fromRemoteJSON(json: any) {
public isRemote = false;
static fromRemoteJSON(json: object) {
const operator = this.fromJSON(json);
operator.isRemote = true;
return operator;
}
static fromJSON(json: any) {
static fromJSON(json: {
plugin_name: string;
_builtin: boolean;
config: object;
}) {
const config = OperatorConfig.fromJSON(json.config);
const operator = new Operator(json.plugin_name, json._builtin, config);
return operator;
Expand Down