From bf351243dfa69095699596e8828904fda025d45c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Fri, 7 Jul 2023 16:43:45 +0200 Subject: [PATCH] fix(Code Node): Install python modules always in a user-writable folder (#6568) * upgrade pyodide * install pyodide modules to a custom user-writable path * in `augmentObject` `newData` is never undefined --- package.json | 3 +- packages/core/src/NodeExecuteFunctions.ts | 4 ++ packages/nodes-base/nodes/Code/Code.node.ts | 4 +- packages/nodes-base/nodes/Code/Pyodide.ts | 25 +++------ .../nodes-base/nodes/Code/PythonSandbox.ts | 53 +++++++------------ packages/nodes-base/nodes/Code/Sandbox.ts | 2 +- .../descriptions/PythonCodeDescription.ts | 15 ------ packages/nodes-base/package.json | 2 +- packages/workflow/src/AugmentObject.ts | 6 ++- packages/workflow/src/Interfaces.ts | 1 + packages/workflow/src/ObservableObject.ts | 3 ++ packages/workflow/src/WorkflowDataProxy.ts | 11 ++++ packages/workflow/test/AugmentObject.test.ts | 14 +++++ patches/pyodide@0.23.4.patch | 20 +++++++ pnpm-lock.yaml | 12 +++-- 15 files changed, 98 insertions(+), 77 deletions(-) create mode 100644 patches/pyodide@0.23.4.patch diff --git a/package.json b/package.json index 1e74db3056..1e9468bfeb 100644 --- a/package.json +++ b/package.json @@ -93,7 +93,8 @@ "element-ui@2.15.12": "patches/element-ui@2.15.12.patch", "typedi@0.10.0": "patches/typedi@0.10.0.patch", "@sentry/cli@2.17.0": "patches/@sentry__cli@2.17.0.patch", - "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch" + "pkce-challenge@3.0.0": "patches/pkce-challenge@3.0.0.patch", + "pyodide@0.23.4": "patches/pyodide@0.23.4.patch" } } } diff --git a/packages/core/src/NodeExecuteFunctions.ts b/packages/core/src/NodeExecuteFunctions.ts index 9e1a4d8b17..57a655d235 100644 --- a/packages/core/src/NodeExecuteFunctions.ts +++ b/packages/core/src/NodeExecuteFunctions.ts @@ -128,6 +128,7 @@ import { setAllWorkflowExecutionMetadata, setWorkflowExecutionMetadata, } from './WorkflowExecutionMetadata'; +import { getUserN8nFolderPath } from './UserSettings'; axios.defaults.timeout = 300000; // Prevent axios from adding x-form-www-urlencoded headers by default @@ -2245,6 +2246,9 @@ const getFileSystemHelperFunctions = (node: INode): FileSystemHelperFunctions => } return createReadStream(filePath); }, + getStoragePath() { + return path.join(getUserN8nFolderPath(), `storage/${node.type}`); + }, }); const getNodeHelperFunctions = ({ diff --git a/packages/nodes-base/nodes/Code/Code.node.ts b/packages/nodes-base/nodes/Code/Code.node.ts index e524a30bad..f7712364d3 100644 --- a/packages/nodes-base/nodes/Code/Code.node.ts +++ b/packages/nodes-base/nodes/Code/Code.node.ts @@ -108,10 +108,8 @@ export class Code implements INodeType { } if (language === 'python') { - const modules = this.getNodeParameter('modules', index) as string; - const moduleImports: string[] = modules ? modules.split(',').map((m) => m.trim()) : []; context.printOverwrite = workflowMode === 'manual' ? this.sendMessageToUI : null; - return new PythonSandbox(context, code, moduleImports, index, this.helpers); + return new PythonSandbox(context, code, index, this.helpers); } else { const sandbox = new JavaScriptSandbox(context, code, index, workflowMode, this.helpers); if (workflowMode === 'manual') { diff --git a/packages/nodes-base/nodes/Code/Pyodide.ts b/packages/nodes-base/nodes/Code/Pyodide.ts index a2578efeae..23bcabbde8 100644 --- a/packages/nodes-base/nodes/Code/Pyodide.ts +++ b/packages/nodes-base/nodes/Code/Pyodide.ts @@ -2,26 +2,15 @@ import type { PyodideInterface } from 'pyodide'; let pyodideInstance: PyodideInterface | undefined; -export async function LoadPyodide(): Promise { +export async function LoadPyodide(packageCacheDir: string): Promise { if (pyodideInstance === undefined) { - // TODO: Find better way to suppress warnings - //@ts-ignore - globalThis.Blob = (await import('node:buffer')).Blob; - - // From: https://github.com/nodejs/node/issues/30810 - const { emitWarning } = process; - process.emitWarning = (warning, ...args) => { - if (args[0] === 'ExperimentalWarning') { - return; - } - if (args[0] && typeof args[0] === 'object' && args[0].type === 'ExperimentalWarning') { - return; - } - return emitWarning(warning, ...(args as string[])); - }; - const { loadPyodide } = await import('pyodide'); - pyodideInstance = await loadPyodide(); + pyodideInstance = await loadPyodide({ packageCacheDir }); + + await pyodideInstance.runPythonAsync(` +from _pyodide_core import jsproxy_typedict +from js import Object +`); } return pyodideInstance; diff --git a/packages/nodes-base/nodes/Code/PythonSandbox.ts b/packages/nodes-base/nodes/Code/PythonSandbox.ts index fccff17de9..cda9b1f80e 100644 --- a/packages/nodes-base/nodes/Code/PythonSandbox.ts +++ b/packages/nodes-base/nodes/Code/PythonSandbox.ts @@ -1,5 +1,5 @@ import type { IExecuteFunctions, INodeExecutionData } from 'n8n-workflow'; -import type { PyProxyDict } from 'pyodide'; +import type { PyDict } from 'pyodide/ffi'; import { LoadPyodide } from './Pyodide'; import type { SandboxContext } from './Sandbox'; import { Sandbox } from './Sandbox'; @@ -18,7 +18,6 @@ export class PythonSandbox extends Sandbox { constructor( context: SandboxContext, private pythonCode: string, - private moduleImports: string[], itemIndex: number | undefined, helpers: IExecuteFunctions['helpers'], ) { @@ -51,47 +50,35 @@ export class PythonSandbox extends Sandbox { } private async runCodeInPython() { - // Below workaround from here: - // https://github.com/pyodide/pyodide/discussions/3537#discussioncomment-4864345 - const runCode = ` -from _pyodide_core import jsproxy_typedict -from js import Object -jsproxy_typedict[0] = type(Object.new().as_object_map()) - -if printOverwrite: - print = printOverwrite - -async def __main(): -${this.pythonCode - .split('\n') - .map((line) => ' ' + line) - .join('\n')} -await __main() -`; - const pyodide = await LoadPyodide(); - - const moduleImportsFiltered = this.moduleImports.filter( - (importModule) => !['asyncio', 'pyodide', 'math'].includes(importModule), - ); - - if (moduleImportsFiltered.length) { - await pyodide.loadPackage('micropip'); - const micropip = pyodide.pyimport('micropip'); - await Promise.all( - moduleImportsFiltered.map((importModule) => micropip.install(importModule)), - ); - } + const packageCacheDir = this.helpers.getStoragePath(); + const pyodide = await LoadPyodide(packageCacheDir); let executionResult; try { + await pyodide.runPythonAsync('jsproxy_typedict[0] = type(Object.new().as_object_map())'); + + await pyodide.loadPackagesFromImports(this.pythonCode); + const dict = pyodide.globals.get('dict'); - const globalsDict: PyProxyDict = dict(); + const globalsDict: PyDict = dict(); for (const key of Object.keys(this.context)) { if ((key === '_env' && envAccessBlocked) || key === '_node') continue; const value = this.context[key]; globalsDict.set(key, value); } + await pyodide.runPythonAsync(` +if 'printOverwrite' in globals(): + print = printOverwrite + `); + + const runCode = ` +async def __main(): +${this.pythonCode + .split('\n') + .map((line) => ' ' + line) + .join('\n')} +await __main()`; executionResult = await pyodide.runPythonAsync(runCode, { globals: globalsDict }); globalsDict.destroy(); } catch (error) { diff --git a/packages/nodes-base/nodes/Code/Sandbox.ts b/packages/nodes-base/nodes/Code/Sandbox.ts index 541075262c..8345a1263e 100644 --- a/packages/nodes-base/nodes/Code/Sandbox.ts +++ b/packages/nodes-base/nodes/Code/Sandbox.ts @@ -35,7 +35,7 @@ export abstract class Sandbox { constructor( private textKeys: SandboxTextKeys, protected itemIndex: number | undefined, - private helpers: IExecuteFunctions['helpers'], + protected helpers: IExecuteFunctions['helpers'], ) {} abstract runCodeAllItems(): Promise; diff --git a/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts b/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts index f60066a5e3..319294fc11 100644 --- a/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts +++ b/packages/nodes-base/nodes/Code/descriptions/PythonCodeDescription.ts @@ -45,19 +45,4 @@ export const pythonCodeDescription: INodeProperties[] = [ }, default: '', }, - { - displayName: 'Python Modules', - name: 'modules', - displayOptions: { - show: { - language: ['python'], - }, - }, - type: 'string', - default: '', - placeholder: 'opencv-python', - description: - 'Comma-separated list of Python modules to load. They have to be installed to be able to be loaded and imported.', - noDataExpression: true, - }, ]; diff --git a/packages/nodes-base/package.json b/packages/nodes-base/package.json index 3e06909b89..95ec20661d 100644 --- a/packages/nodes-base/package.json +++ b/packages/nodes-base/package.json @@ -831,7 +831,7 @@ "pg-promise": "^10.5.8", "pretty-bytes": "^5.6.0", "promise-ftp": "^1.3.5", - "pyodide": "^0.22.1", + "pyodide": "^0.23.4", "redis": "^3.1.1", "rhea": "^1.0.11", "rss-parser": "^3.7.0", diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts index 27602b3a23..2b35fb4d74 100644 --- a/packages/workflow/src/AugmentObject.ts +++ b/packages/workflow/src/AugmentObject.ts @@ -133,7 +133,11 @@ export function augmentObject(data: T): T { return true; }, - + has(target, key) { + if (deletedProperties.indexOf(key) !== -1) return false; + const newKeys = Object.keys(newData); + return Reflect.has(newKeys.length ? newData : target, key); + }, ownKeys(target) { const originalKeys = Reflect.ownKeys(target); const newKeys = Object.keys(newData); diff --git a/packages/workflow/src/Interfaces.ts b/packages/workflow/src/Interfaces.ts index 7b7c098a69..ecf059438a 100644 --- a/packages/workflow/src/Interfaces.ts +++ b/packages/workflow/src/Interfaces.ts @@ -671,6 +671,7 @@ interface JsonHelperFunctions { export interface FileSystemHelperFunctions { createReadStream(path: PathLike): Promise; + getStoragePath(): string; } export interface BinaryHelperFunctions { diff --git a/packages/workflow/src/ObservableObject.ts b/packages/workflow/src/ObservableObject.ts index 4f97de1a19..d5d0c8f422 100644 --- a/packages/workflow/src/ObservableObject.ts +++ b/packages/workflow/src/ObservableObject.ts @@ -51,6 +51,9 @@ export function create( get(target, name, receiver) { return Reflect.get(target, name, receiver); }, + has(target, key) { + return Reflect.has(target, key); + }, set(target, name, value) { if (parent === undefined) { // If no parent is given mark current data as changed diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index 03d4ffc40f..68559fda2b 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -146,6 +146,7 @@ export class WorkflowDataProxy { return new Proxy( {}, { + has: () => true, ownKeys(target) { if (Reflect.ownKeys(target).length === 0) { // Target object did not get set yet @@ -178,6 +179,7 @@ export class WorkflowDataProxy { return new Proxy( {}, { + has: () => true, ownKeys(target) { return Reflect.ownKeys(target); }, @@ -202,6 +204,7 @@ export class WorkflowDataProxy { const node = this.workflow.nodes[nodeName]; return new Proxy(node.parameters, { + has: () => true, ownKeys(target) { return Reflect.ownKeys(target); }, @@ -384,6 +387,7 @@ export class WorkflowDataProxy { return new Proxy( { binary: undefined, data: undefined, json: undefined }, { + has: () => true, get(target, name, receiver) { if (name === 'isProxy') return true; name = name.toString(); @@ -461,6 +465,7 @@ export class WorkflowDataProxy { return new Proxy( {}, { + has: () => true, get(target, name, receiver) { if (name === 'isProxy') return true; @@ -491,6 +496,7 @@ export class WorkflowDataProxy { return new Proxy( {}, { + has: () => true, ownKeys(target) { return allowedValues; }, @@ -538,6 +544,7 @@ export class WorkflowDataProxy { return new Proxy( {}, { + has: () => true, ownKeys(target) { return allowedValues; }, @@ -580,6 +587,7 @@ export class WorkflowDataProxy { return new Proxy( {}, { + has: () => true, get(target, name, receiver) { if (name === 'isProxy') return true; @@ -950,6 +958,7 @@ export class WorkflowDataProxy { return new Proxy( {}, { + has: () => true, ownKeys(target) { return [ 'pairedItem', @@ -1073,6 +1082,7 @@ export class WorkflowDataProxy { }, $input: new Proxy({} as ProxyInput, { + has: () => true, ownKeys(target) { return ['all', 'context', 'first', 'item', 'last', 'params']; }, @@ -1238,6 +1248,7 @@ export class WorkflowDataProxy { }; return new Proxy(base, { + has: () => true, get(target, name, receiver) { if (name === 'isProxy') return true; diff --git a/packages/workflow/test/AugmentObject.test.ts b/packages/workflow/test/AugmentObject.test.ts index b9871d347d..ab208f6c21 100644 --- a/packages/workflow/test/AugmentObject.test.ts +++ b/packages/workflow/test/AugmentObject.test.ts @@ -557,5 +557,19 @@ describe('AugmentObject', () => { writable: true, }); }); + + test('should return valid values on `has` calls', () => { + const originalObject = { + x: { + y: {}, + }, + }; + const augmentedObject = augmentObject(originalObject); + expect('y' in augmentedObject.x).toBe(true); + expect('z' in augmentedObject.x).toBe(false); + + augmentedObject.x.z = 5; + expect('z' in augmentedObject.x).toBe(true); + }); }); }); diff --git a/patches/pyodide@0.23.4.patch b/patches/pyodide@0.23.4.patch new file mode 100644 index 0000000000..efb5318177 --- /dev/null +++ b/patches/pyodide@0.23.4.patch @@ -0,0 +1,20 @@ +diff --git a/pyodide.d.ts b/pyodide.d.ts +index d5ed46f6345855a75ec6f2b7ef73237a0af64a7e..e0087c83792558ca30713e9d07a9a37625f68d8d 100644 +--- a/pyodide.d.ts ++++ b/pyodide.d.ts +@@ -1118,6 +1118,15 @@ export declare function loadPyodide(options?: { + * (``pyodide.js`` or ``pyodide.mjs``) removed. + */ + indexURL?: string; ++ /** ++ * The file path where packages will be cached in `node.js`. If a package ++ * exists in `packageCacheDir` it is loaded from there, otherwise it is ++ * downloaded from the JsDelivr CDN and then cached into `packageCacheDir`. ++ * Only applies when running in node.js. Ignored in browsers. ++ * ++ * Default: same as indexURL ++ */ ++ packageCacheDir?: string; + /** + * file. You can produce custom lock files with :py:func:`micropip.freeze`. + * Default: ```${indexURL}/repodata.json``` \ No newline at end of file diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index eb5e00a0de..5c911ca83c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -35,6 +35,9 @@ patchedDependencies: pkce-challenge@3.0.0: hash: dypouzb3lve7vncq25i5fuanki path: patches/pkce-challenge@3.0.0.patch + pyodide@0.23.4: + hash: kzcwsjcayy5m6iezu7r4tdimjq + path: patches/pyodide@0.23.4.patch typedi@0.10.0: hash: 62r6bc2crgimafeyruodhqlgo4 path: patches/typedi@0.10.0.patch @@ -1142,8 +1145,8 @@ importers: specifier: ^1.3.5 version: 1.3.5(promise-ftp-common@1.1.5) pyodide: - specifier: ^0.22.1 - version: 0.22.1 + specifier: ^0.23.4 + version: 0.23.4(patch_hash=kzcwsjcayy5m6iezu7r4tdimjq) redis: specifier: ^3.1.1 version: 3.1.2 @@ -18656,8 +18659,8 @@ packages: resolution: {integrity: sha512-t+x1zEHDjBwkDGY5v5ApnZ/utcd4XYDiJsaQQoptTXgUXX95sDg1elCdJghzicm7n2mbCBJ3uYWr6M22SO19rg==} dev: true - /pyodide@0.22.1: - resolution: {integrity: sha512-6+PkFLTC+kcBKtFQxYBxR44J5IBxLm8UGkobLgZv1SxzV9qOU2rb0YYf0qDtlnfDiN/IQd2uckf+D8Zwe88Mqg==} + /pyodide@0.23.4(patch_hash=kzcwsjcayy5m6iezu7r4tdimjq): + resolution: {integrity: sha512-WpQUHaIXQ1xede5BMqPAjBcmopxN22s5hEsYOR8T7/UW/fkNLFUn07SaemUgthbtvedD5JGymMMj4VpD9sGMTg==} dependencies: base-64: 1.0.0 node-fetch: 2.6.8 @@ -18667,6 +18670,7 @@ packages: - encoding - utf-8-validate dev: false + patched: true /python-struct@1.1.3: resolution: {integrity: sha512-UsI/mNvk25jRpGKYI38Nfbv84z48oiIWwG67DLVvjRhy8B/0aIK+5Ju5WOHgw/o9rnEmbAS00v4rgKFQeC332Q==}