Skip to content

Commit

Permalink
Merge pull request #277 from flatironinstitute/revert-274-cancellable…
Browse files Browse the repository at this point in the history
…-scripts

Revert "Allow scripts to be cancelable"
  • Loading branch information
WardBrian authored Feb 12, 2025
2 parents 1abf138 + 0c7679d commit f506bb0
Show file tree
Hide file tree
Showing 12 changed files with 47 additions and 165 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const DataPyPanel: FunctionComponent = () => {
[consoleRef, onData, onStatus],
);

const { run, cancel } = usePyodideWorker(callbacks);
const { run } = usePyodideWorker(callbacks);

const handleRun = useCallback(
(code: string) => {
Expand Down Expand Up @@ -62,7 +62,6 @@ const DataPyPanel: FunctionComponent = () => {
language="python"
status={status}
onRun={handleRun}
onCancel={cancel}
runnable={true}
notRunnableReason=""
onHelp={handleHelp}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ const handleHelp = () =>
const DataRPanel: FunctionComponent = () => {
const { consoleRef, status, onStatus, onData } = useDataGenState("r");

const { run, cancel } = useWebR({ consoleRef, onStatus, onData });

const { run } = useWebR({ consoleRef, onStatus, onData });
const handleRun = useCallback(
async (code: string) => {
clearOutputDivs(consoleRef);
Expand All @@ -41,7 +40,6 @@ const DataRPanel: FunctionComponent = () => {
language="r"
status={status}
onRun={handleRun}
onCancel={cancel}
runnable={true}
notRunnableReason=""
onHelp={handleHelp}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const AnalysisPyPanel: FunctionComponent<NeedsLatestRun> = ({ latestRun }) => {
[consoleRef, imagesRef, onStatus],
);

const { run, cancel } = usePyodideWorker(callbacks);
const { run } = usePyodideWorker(callbacks);

const handleRun = useCallback(
(code: string) => {
Expand Down Expand Up @@ -69,7 +69,6 @@ const AnalysisPyPanel: FunctionComponent<NeedsLatestRun> = ({ latestRun }) => {
language="python"
status={status}
onRun={handleRun}
onCancel={cancel}
runnable={runnable}
notRunnableReason={notRunnableReason}
imagesRef={imagesRef}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const AnalysisRPanel: FunctionComponent<NeedsLatestRun> = ({ latestRun }) => {
files,
} = useAnalysisState(latestRun);

const { run, cancel } = useWebR({ consoleRef, imagesRef, onStatus });
const { run } = useWebR({ consoleRef, imagesRef, onStatus });

const handleRun = useCallback(
async (userCode: string) => {
Expand All @@ -48,7 +48,6 @@ const AnalysisRPanel: FunctionComponent<NeedsLatestRun> = ({ latestRun }) => {
language="r"
status={status}
onRun={handleRun}
onCancel={cancel}
runnable={runnable}
notRunnableReason={notRunnableReason}
imagesRef={imagesRef}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@ const DrawsTablePanel: FunctionComponent<DrawsTableProps> = ({
<TableCell key="chain">Chain</TableCell>
<TableCell key="draw">Draw</TableCell>
{paramNames.map((name, i) => (
<TableCell padding="checkbox" key={i}>
{name}
</TableCell>
<TableCell key={i}>{name}</TableCell>
))}
</TableRow>
</SuccessColoredTableHead>
Expand All @@ -107,9 +105,7 @@ const DrawsTablePanel: FunctionComponent<DrawsTableProps> = ({
<TableCell>{drawChainIds[i]}</TableCell>
<TableCell>{drawNumbers[i]}</TableCell>
{formattedDraws.map((draw, j) => (
<TableCell padding="checkbox" key={j}>
{draw[i]}
</TableCell>
<TableCell key={j}>{draw[i]}</TableCell>
))}
</SuccessBorderedTableRow>
))}
Expand Down
41 changes: 6 additions & 35 deletions gui/src/app/components/FileEditor/ScriptEditor.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { FunctionComponent, RefObject, useCallback, use, useMemo } from "react";

import { Close, Help, PlayArrow } from "@mui/icons-material";
import { Help, PlayArrow } from "@mui/icons-material";
import Box from "@mui/material/Box";
import { Split } from "@geoffcox/react-splitter";
import { useMonaco } from "@monaco-editor/react";
Expand All @@ -22,7 +22,6 @@ export type ScriptEditorProps = {
filename: FileNames;
dataKey: ProjectKnownFiles;
onRun: (code: string) => void;
onCancel?: () => void;
runnable: boolean;
notRunnableReason?: string;
onHelp?: () => void;
Expand All @@ -36,7 +35,6 @@ const ScriptEditor: FunctionComponent<ScriptEditorProps> = ({
filename,
dataKey,
onRun,
onCancel,
runnable,
notRunnableReason,
onHelp,
Expand Down Expand Up @@ -76,12 +74,11 @@ const ScriptEditor: FunctionComponent<ScriptEditorProps> = ({

const monacoInstance = useMonaco();

const scriptShortcuts: editor.IActionDescriptor[] = useMemo(() => {
const runCtrlEnter: editor.IActionDescriptor[] = useMemo(() => {
if (!monacoInstance) {
return [];
}
const actions = [
// Ctrl-Enter to run
return [
{
id: "run-script",
label: "Run Script",
Expand All @@ -95,19 +92,7 @@ const ScriptEditor: FunctionComponent<ScriptEditorProps> = ({
},
},
];
if (onCancel) {
// Ctrl-C to cancel
actions.push({
id: "cancel-script",
label: "Cancel Script",
keybindings: [
monacoInstance.KeyMod.CtrlCmd | monacoInstance.KeyCode.KeyC,
],
run: onCancel,
});
}
return actions;
}, [monacoInstance, onCancel, runCode, runnable, unsavedChanges]);
}, [monacoInstance, runCode, runnable, unsavedChanges]);

const toolbarItems: ToolbarItem[] = useMemo(() => {
return makeToolbar({
Expand All @@ -116,13 +101,11 @@ const ScriptEditor: FunctionComponent<ScriptEditorProps> = ({
runnable: runnable && !unsavedChanges,
notRunnableReason,
onRun: runCode,
onCancel,
onHelp,
});
}, [
language,
notRunnableReason,
onCancel,
onHelp,
runCode,
runnable,
Expand All @@ -141,7 +124,7 @@ const ScriptEditor: FunctionComponent<ScriptEditorProps> = ({
onSaveText={onSaveText}
toolbarItems={toolbarItems}
contentOnEmpty={contentOnEmpty}
actions={scriptShortcuts}
actions={runCtrlEnter}
/>
<ConsoleOutputPanel consoleRef={consoleRef} />
</Split>
Expand All @@ -155,9 +138,8 @@ const makeToolbar = (o: {
notRunnableReason?: string;
onRun: () => void;
onHelp?: () => void;
onCancel?: () => void;
}): ToolbarItem[] => {
const { status, onRun, runnable, onHelp, name, onCancel } = o;
const { status, onRun, runnable, onHelp, name } = o;
const ret: ToolbarItem[] = [];
if (onHelp !== undefined) {
ret.push({
Expand All @@ -184,17 +166,6 @@ const makeToolbar = (o: {
});
}

if (onCancel && status === "running") {
ret.push({
type: "button",
tooltip: "Cancel",
label: "Cancel",
icon: <Close />,
onClick: onCancel,
color: "error",
});
}

let label: string;
let color: ColorOptions;
if (status === "loading") {
Expand Down
76 changes: 28 additions & 48 deletions gui/src/app/core/Scripting/pyodide/pyodideWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,33 @@ import { isMonacoWorkerNoise } from "@SpUtil/isMonacoWorkerNoise";
import { InterpreterStatus } from "@SpCore/Scripting/InterpreterTypes";
import {
MessageFromPyodideWorker,
MessageToPyodideWorker,
PyodideRunSettings,
} from "./pyodideWorkerTypes";
import spDrawsScript from "./sp_load_draws.py?raw";
import spMPLScript from "./sp_patch_matplotlib.py?raw";

let pyodide: PyodideInterface | null = null;
const loadPyodideInstance = async () => {
const pyodide = await loadPyodide({
indexURL: "https://cdn.jsdelivr.net/pyodide/v0.27.2/full",
stdout: (x: string) => {
sendStdout(x);
},
stderr: (x: string) => {
sendStderr(x);
},
packages: ["numpy", "micropip", "pandas"],
});
console.log("pyodide loaded");

pyodide.FS.writeFile("sp_load_draws.py", spDrawsScript);
pyodide.FS.writeFile("sp_patch_matplotlib.py", spMPLScript);

return pyodide;
if (pyodide === null) {
pyodide = await loadPyodide({
indexURL: "https://cdn.jsdelivr.net/pyodide/v0.27.2/full",
stdout: (x: string) => {
sendStdout(x);
},
stderr: (x: string) => {
sendStderr(x);
},
packages: ["numpy", "micropip", "pandas"],
});
setStatus("installing");

pyodide.FS.writeFile("sp_load_draws.py", spDrawsScript);
pyodide.FS.writeFile("sp_patch_matplotlib.py", spMPLScript);

return pyodide;
} else {
return pyodide;
}
};

const sendMessageToMain = (message: MessageFromPyodideWorker) => {
Expand All @@ -52,38 +56,25 @@ const addImage = (image: any) => {
sendMessageToMain({ type: "addImage", image });
};

self.onmessage = async (e: MessageEvent<MessageToPyodideWorker>) => {
console.log("pyodide worker loaded");

self.onmessage = async (e) => {
if (isMonacoWorkerNoise(e.data)) {
return;
}
const message = e.data;
await run(
message.code,
message.spData,
message.spRunSettings,
message.files,
message.interruptBuffer,
);
await run(message.code, message.spData, message.spRunSettings, message.files);
};
console.log("pyodide worker initialized");

console.log("opportunistically loading pyodide");
const pyodidePromise: Promise<PyodideInterface> = loadPyodideInstance();

const run = async (
code: string,
spData: Record<string, any> | undefined,
spPySettings: PyodideRunSettings,
files: Record<string, string> | undefined,
interruptBuffer: Uint8Array | undefined,
) => {
setStatus("loading");
try {
const pyodide = await pyodidePromise;
if (interruptBuffer) {
pyodide.setInterruptBuffer(interruptBuffer);
}
setStatus("installing");
const pyodide = await loadPyodideInstance();

const [scriptPreamble, scriptPostamble] = getScriptParts(spPySettings);

Expand All @@ -104,7 +95,6 @@ const run = async (
let succeeded = false;
try {
const packageFutures = [];
let patch_http = false;
const micropip = pyodide.pyimport("micropip");

if (spPySettings.showsPlots) {
Expand All @@ -114,20 +104,10 @@ const run = async (
packageFutures.push(micropip.install("arviz"));
}
}
if (script.includes("requests")) {
patch_http = true;
packageFutures.push(
micropip.install(["requests", "lzma", "pyodide-http"]),
);
}
packageFutures.push(micropip.install("stanio"));
packageFutures.push(pyodide.loadPackagesFromImports(script));
await Promise.all(packageFutures);
if (patch_http) {
await pyodide.runPythonAsync(`
from pyodide_http import patch_all
patch_all()
`);
for (const f of packageFutures) {
await f;
}

if (files) {
Expand Down
1 change: 0 additions & 1 deletion gui/src/app/core/Scripting/pyodide/pyodideWorkerTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ export type MessageToPyodideWorker = {
spData: Record<string, any> | undefined;
spRunSettings: PyodideRunSettings;
files: Record<string, string> | undefined;
interruptBuffer: Uint8Array | undefined;
};

export const isMessageToPyodideWorker = (
Expand Down
29 changes: 2 additions & 27 deletions gui/src/app/core/Scripting/pyodide/usePyodideWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ type RunPyProps = {

class PyodideWorkerInterface {
#worker: Worker | undefined;
#interruptBuffer: Uint8Array | undefined;

private constructor(private callbacks: PyodideWorkerCallbacks) {
// do not call this directly, use create() instead
Expand All @@ -40,15 +39,6 @@ class PyodideWorkerInterface {
type: "module",
});

if (window.crossOriginIsolated) {
this.#interruptBuffer = new Uint8Array(new SharedArrayBuffer(1));
} else {
console.warn(
"SharedArrayBuffer is not available, interrupting the Pyodide worker will not work",
);
this.#interruptBuffer = undefined;
}

this.#worker.onmessage = (e: MessageEvent) => {
const msg = e.data;
if (!isMessageFromPyodideWorker(msg)) {
Expand Down Expand Up @@ -95,32 +85,17 @@ class PyodideWorkerInterface {
spData,
spRunSettings,
files,
interruptBuffer: this.#interruptBuffer,
};
if (this.#worker) {
if (this.#interruptBuffer) {
// clear in case previous run was interrupted
this.#interruptBuffer[0] = 0;
}
this.#worker.postMessage(msg);
} else {
throw new Error("pyodide worker is not defined");
}
}

cancel() {
if (this.#interruptBuffer && this.#interruptBuffer[0] === 0) {
// SIGINT
this.#interruptBuffer[0] = 2;
} else {
// if the interrupt buffer doesn't exist, or has already been set
// (and the user is requesting cancellation still)
// we can just terminate the worker
this.#worker?.terminate();
this.callbacks.onStatus("failed");
this.callbacks.onStderr("Python execution cancelled by user");
this.#initialize();
}
this.#worker?.terminate();
this.#initialize();
}
}

Expand Down
Loading

0 comments on commit f506bb0

Please sign in to comment.