diff --git a/backend/kale/compiler.py b/backend/kale/compiler.py index 0cf4903d..b7c1b8e7 100644 --- a/backend/kale/compiler.py +++ b/backend/kale/compiler.py @@ -103,6 +103,10 @@ def generate_dsl(self): Returns (str): A Python executable script """ + # Fail early if there are no steps in the pipeline. + if not hasattr(self.pipeline, "steps") or not self.pipeline.steps: + raise ValueError("Task is missing from pipeline.") + # List of lightweight components generated code lightweight_components = [ self.generate_lightweight_component(step) for step in self.pipeline.steps diff --git a/backend/kale/rpc/nb.py b/backend/kale/rpc/nb.py index 16f5d725..c0089991 100644 --- a/backend/kale/rpc/nb.py +++ b/backend/kale/rpc/nb.py @@ -20,7 +20,7 @@ from kale import Compiler, NotebookProcessor, marshal from kale.common import astutils, kfputils, kfutils, podutils -from kale.rpc.errors import RPCInternalError +from kale.rpc.errors import RPCInternalError, RPCUnhandledError from kale.rpc.log import create_adapter KALE_MARSHAL_DIR_POSTFIX = ".kale.marshal.dir" @@ -95,20 +95,43 @@ def get_base_image(request): # fixme: Remove the debug argument from the labextension RPC call. def compile_notebook(request, source_notebook_path, notebook_metadata_overrides=None, debug=False): """Compile the notebook to KFP DSL.""" - processor = NotebookProcessor(source_notebook_path, notebook_metadata_overrides) - pipeline = processor.run() - imports_and_functions = processor.get_imports_and_functions() - script_path = Compiler(pipeline, imports_and_functions).compile() - # FIXME: Why were we tapping into the Kale logger? - # instance = Kale(source_notebook_path, notebook_metadata_overrides, debug) - # instance.logger = request.log if hasattr(request, "log") else logger - - package_path = kfputils.compile_pipeline(script_path, pipeline.config.pipeline_name) - - return { - "pipeline_package_path": os.path.relpath(package_path), - "pipeline_metadata": pipeline.config.to_dict(), - } + try: + processor = NotebookProcessor(source_notebook_path, notebook_metadata_overrides) + pipeline = processor.run() + imports_and_functions = processor.get_imports_and_functions() + script_path = Compiler(pipeline, imports_and_functions).compile() + + """FIXME: Why were we tapping into the Kale logger? + instance = Kale(source_notebook_path, + notebook_metadata_overrides, debug) + instance.logger = request.log if + hasattr(request, "log") else logger""" + + package_path = kfputils.compile_pipeline(script_path, pipeline.config.pipeline_name) + + return { + "pipeline_package_path": os.path.relpath(package_path), + "pipeline_metadata": pipeline.config.to_dict(), + } + except ValueError as e: + msg = str(e) + request.log.exception("ValueError during notebook compilation: %s", msg) + if "Task is missing from pipeline" in msg: + # Provide guidance to the user about how to fix the issue. + raise RPCUnhandledError( + details=( + "The pipeline does not have any steps. " + "Please tag a cell as a pipeline step or set " + "`steps_defaults` in the notebook metadata." + ), + trans_id=request.trans_id, + ) + raise RPCInternalError(details=msg, trans_id=request.trans_id) + except Exception as e: + # Let the run dispatcher handle generic exceptions as unhandled, + # but log for debug purposes. + request.log.exception("Unexpected error during notebook compilation: %s", e) + raise def validate_notebook(request, source_notebook_path, notebook_metadata_overrides=None): diff --git a/examples/base/candies_sharing.ipynb b/examples/base/candies_sharing.ipynb index 4989ede6..b128086b 100644 --- a/examples/base/candies_sharing.ipynb +++ b/examples/base/candies_sharing.ipynb @@ -207,7 +207,7 @@ "name": "python", "nbconvert_exporter": "python", "pygments_lexer": "ipython3", - "version": "3.10.18" + "version": "3.10.19" } }, "nbformat": 4, diff --git a/labextension/src/lib/RPCUtils.tsx b/labextension/src/lib/RPCUtils.tsx index e90bb25e..b0c7dca2 100644 --- a/labextension/src/lib/RPCUtils.tsx +++ b/labextension/src/lib/RPCUtils.tsx @@ -16,6 +16,8 @@ import * as React from 'react'; import { NotebookPanel } from '@jupyterlab/notebook'; import { Kernel } from '@jupyterlab/services'; import NotebookUtils from './NotebookUtils'; +// @ts-expect-error This module is not typed +import SanitizedHTML from 'react-sanitized-html'; import { isError, IError, IOutput } from '@jupyterlab/nbformat'; import { Notification } from '@jupyterlab/apputils'; @@ -30,11 +32,11 @@ export const globalUnhandledRejection = async (event: any) => { // isolate the segments const stackLines = errorStack.split('\n'); // create alert string - const alert_string = 'Unhandled Error' + const alert_string = 'Unhandled Error'; // call the toast pop up - if (errorStack.includes('lab/extensions/')){ + if (errorStack.includes('lab/extensions/')) { // if the error is caused by a jupyterlab extension, try to isolate the extension name - const extensionName = getExtensionName(stackLines) + const extensionName = getExtensionName(stackLines); Notification.error(`An unhandled error has been thrown.`, { actions: [ { label: 'Details', callback: () => NotebookUtils.showMessage(alert_string, @@ -46,10 +48,18 @@ export const globalUnhandledRejection = async (event: any) => { autoClose: 3000 }); } else { + const extensionName = getExtensionName(stackLines); Notification.error(`An unhandled error has been thrown.`, { actions: [ - { label: 'Details', callback: () => NotebookUtils.showMessage(alert_string, - ["Please see console for more details."]) } + { + label: 'Details', + callback: () => + NotebookUtils.showMessage(alert_string, [ + 'An unhandled error was thrown from:', + extensionName, + 'Please see console for more details.' + ]) + } ], autoClose: 3000 }); @@ -57,9 +67,9 @@ export const globalUnhandledRejection = async (event: any) => { } }; -function getExtensionName(stackLines: Array){ - const urlSplit = stackLines.slice(1,2).toString(); - const extensionSplit = urlSplit.split('@').slice(1,2).toString(); +function getExtensionName(stackLines: Array) { + const urlSplit = stackLines.slice(1, 2).toString(); + const extensionSplit = urlSplit.split('@').slice(1, 2).toString(); const extensionParts = extensionSplit.split('/'); extensionParts.pop(); const extensionName = extensionParts.join('/'); @@ -82,7 +92,7 @@ export enum RPC_CALL_STATUS { NotFound = 3, InternalError = 4, ServiceUnavailable = 5, - UnhandledError = 6, + UnhandledError = 6 } const getRpcCodeName = (code: number) => { @@ -104,6 +114,8 @@ const getRpcCodeName = (code: number) => { } }; + + export const rokErrorTooltip = (rokError: IRPCError) => { return ( @@ -132,12 +144,12 @@ export const executeRpc = async ( env: Kernel.IKernelConnection | NotebookPanel, func: string, kwargs: any = {}, - ctx: { nb_path: string | null } = { nb_path: null }, + ctx: { nb_path: string | null } = { nb_path: null } ) => { const cmd: string = 'from kale.rpc.run import run as __kale_rpc_run\n' + `__kale_rpc_result = __kale_rpc_run("${func}", '${serialize( - kwargs, + kwargs )}', '${serialize(ctx)}')`; console.log('Executing command: ' + cmd); const expressions = { result: '__kale_rpc_result' }; @@ -148,7 +160,7 @@ export const executeRpc = async ( ? await NotebookUtils.sendKernelRequestFromNotebook( env, cmd, - expressions, + expressions ) : await NotebookUtils.sendKernelRequest(env, cmd, expressions); } catch (e) { @@ -158,7 +170,7 @@ export const executeRpc = async ( const error = { rpc: `${func}`, status: `${(e as IError).ename}: ${(e as IError).evalue}`, - output: (e as IError).traceback, + output: (e as IError).traceback }; throw new KernelError(error); } @@ -174,7 +186,7 @@ export const executeRpc = async ( const error = { rpc: `${func}`, status: output.result.status, - output: output, + output: output }; throw new KernelError(error); } @@ -193,7 +205,7 @@ export const executeRpc = async ( rpc: `${func}`, err_message: 'Failed to parse response as JSON', error: error, - jsonData: json_data, + jsonData: json_data }; throw new JSONParseError(jsonError); } @@ -205,12 +217,11 @@ export const executeRpc = async ( err_message: parsedResult.err_message, err_details: parsedResult.err_details, err_cls: parsedResult.err_cls, - trans_id: parsedResult.trans_id, + trans_id: parsedResult.trans_id }; throw new RPCError(error); } return parsedResult.result; - }; export const showError = async ( @@ -221,11 +232,11 @@ export const showError = async ( refresh: boolean = true, method: string | null = null, code: number | null = null, - trans_id: number | null = null, + trans_id: number | null = null ): Promise => { const msg: string[] = [ `Browser: ${navigator ? navigator.userAgent : 'other'}`, - `Type: ${type}`, + `Type: ${type}` ]; if (method) { msg.push(`Method: ${method}()`); @@ -247,8 +258,9 @@ export const showError = async ( export const showRpcError = async ( error: IRPCError, - refresh: boolean = false, + refresh: boolean = false ): Promise => { + await showError( 'An RPC Error has occurred', 'RPC', @@ -257,7 +269,7 @@ export const showRpcError = async ( refresh, error.rpc, error.code, - error.trans_id, + error.trans_id ); }; @@ -267,7 +279,7 @@ export const _legacy_executeRpc = async ( kernel: Kernel.IKernelConnection, func: string, args: any = {}, - nb_path: string | null = null, + nb_path: string | null = null ) => { if (!nb_path && notebook) { nb_path = notebook.context.path; @@ -300,7 +312,7 @@ export const _legacy_executeRpcAndShowRPCError = async ( kernel: Kernel.IKernelConnection, func: string, args: any = {}, - nb_path: string | null = null, + nb_path: string | null = null ) => { try { const result = await _legacy_executeRpc( @@ -308,7 +320,7 @@ export const _legacy_executeRpcAndShowRPCError = async ( kernel, func, args, - nb_path, + nb_path ); return result; } catch (error) { @@ -321,7 +333,10 @@ export const _legacy_executeRpcAndShowRPCError = async ( }; export abstract class BaseError extends Error { - constructor(message: string, public error: any) { + constructor( + message: string, + public error: any + ) { super(message); this.name = this.constructor.name; this.stack = new Error(message).stack; @@ -345,7 +360,7 @@ export class KernelError extends BaseError { this.error.status, JSON.stringify(this.error.output, null, 3), refresh, - this.error.rpc, + this.error.rpc ); } } @@ -363,7 +378,7 @@ export class JSONParseError extends BaseError { this.error.error.message, this.error.json_data, refresh, - this.error.rpc, + this.error.rpc ); } } @@ -378,3 +393,4 @@ export class RPCError extends BaseError { await showRpcError(this.error, refresh); } } + diff --git a/labextension/src/widgets/LeftPanel.tsx b/labextension/src/widgets/LeftPanel.tsx index b09655d5..c56a785e 100644 --- a/labextension/src/widgets/LeftPanel.tsx +++ b/labextension/src/widgets/LeftPanel.tsx @@ -104,6 +104,43 @@ export class KubeflowKaleLeftPanel extends React.Component { // init state default values state = DefaultState; + // Return the notebook file name without extension (e.g. 'MyNotebook' from 'path/to/MyNotebook.ipynb') + getNotebookFileName = (notebook: NotebookPanel | null): string => { + if (!notebook || !notebook.context || !notebook.context.path) { + return ''; + } + const path = notebook.context.path as string; + const base = path.split('/').pop() || ''; + return base.replace(/\.ipynb$/i, ''); + }; + + // Sanitize a name to match the pipeline name regex: + // '^[a-z0-9]([-a-z0-9]*[a-z0-9])?$' + // Steps: + // - lowercase + // - replace invalid chars with '-' + // - collapse multiple '-' into one + // - trim leading/trailing '-' + // - if result is empty, return a fallback unique name + sanitizePipelineName = (name: string): string => { + if (!name) { + return ''; + } + let s = name.toLowerCase(); + // replace any char that is not [a-z0-9-] with '-' + s = s.replace(/[^a-z0-9-]+/g, '-'); + // collapse multiple hyphens + s = s.replace(/-+/g, '-'); + // trim leading/trailing hyphens + s = s.replace(/^-+|-+$/g, ''); + // ensure it starts and ends with alphanumeric; if not, fallback + if (!/^[a-z0-9].*[a-z0-9]$/.test(s)) { + // fallback: use a predictable but unique name + return 'pipeline-' + Date.now().toString(36); + } + return s; + }; + getActiveNotebook = (): NotebookPanel | null => { return this.props.tracker.currentWidget; }; @@ -329,11 +366,19 @@ export class KubeflowKaleLeftPanel extends React.Component { } } + // Use notebook filename as default pipeline name when not provided in metadata + const defaultPipelineName = this.getNotebookFileName(notebook); + const sanitizedDefaultPipelineName = + this.sanitizePipelineName(defaultPipelineName); const metadata: IKaleNotebookMetadata = { ...notebookMetadata, experiment: experiment, experiment_name: experiment_name, - pipeline_name: notebookMetadata['pipeline_name'] || '', + pipeline_name: + notebookMetadata['pipeline_name'] && + notebookMetadata['pipeline_name'] !== '' + ? notebookMetadata['pipeline_name'] + : sanitizedDefaultPipelineName, pipeline_description: notebookMetadata['pipeline_description'] || '', base_image: notebookMetadata['base_image'] || DefaultState.metadata.base_image, @@ -343,11 +388,16 @@ export class KubeflowKaleLeftPanel extends React.Component { metadata: metadata, }); } else { + // If no notebook metadata exists, set pipeline_name to the sanitized notebook filename + const defaultPipelineName = this.getNotebookFileName(notebook); + const sanitizedDefaultPipelineName = + this.sanitizePipelineName(defaultPipelineName); this.setState(prevState => ({ metadata: { ...DefaultState.metadata, experiment: prevState.metadata.experiment, experiment_name: prevState.metadata.experiment_name, + pipeline_name: sanitizedDefaultPipelineName, }, })); }