From 1b5fd9d148b5dce5f9f4dd8735b53dc4ecf58bde Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 11:24:08 -0500 Subject: [PATCH 1/9] Fix %%sql cells in %run_shared magic --- singlestoredb/magics/run_shared.py | 74 +++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/singlestoredb/magics/run_shared.py b/singlestoredb/magics/run_shared.py index c021e894c..bb41ec907 100644 --- a/singlestoredb/magics/run_shared.py +++ b/singlestoredb/magics/run_shared.py @@ -1,6 +1,8 @@ import os import tempfile +from pathlib import Path from typing import Any +from warnings import warn from IPython.core.interactiveshell import InteractiveShell from IPython.core.magic import line_magic @@ -8,6 +10,8 @@ from IPython.core.magic import magics_class from IPython.core.magic import needs_local_scope from IPython.core.magic import no_var_expand +from IPython.utils.contexts import preserve_keys +from IPython.utils.syspathcontext import prepended_to_syspath from jinja2 import Template @@ -50,4 +54,72 @@ def run_shared(self, line: str, local_ns: Any = None) -> Any: # Execute the SQL command self.shell.run_line_magic('sql', sql_command) # Run the downloaded file - self.shell.run_line_magic('run', f'"{temp_file_path}"') + with preserve_keys(self.shell.user_ns, '__file__'): + self.shell.user_ns['__file__'] = temp_file_path + self.shell.safe_execfile_ipy(temp_file_path, raise_exceptions=True) + + def safe_execfile_ipy( + self, + fname: str, + shell_futures: bool = False, + raise_exceptions: bool = False, + ) -> None: + """Like safe_execfile, but for .ipy or .ipynb files with IPython syntax. + + Parameters + ---------- + fname : str + The name of the file to execute. The filename must have a + .ipy or .ipynb extension. + shell_futures : bool (False) + If True, the code will share future statements with the interactive + shell. It will both be affected by previous __future__ imports, and + any __future__ imports in the code will affect the shell. If False, + __future__ imports are not shared in either direction. + raise_exceptions : bool (False) + If True raise exceptions everywhere. Meant for testing. + """ + fpath = Path(fname).expanduser().resolve() + + # Make sure we can open the file + try: + with fpath.open('rb'): + pass + except Exception: + warn('Could not open file <%s> for safe execution.' % fpath) + return + + # Find things also in current directory. This is needed to mimic the + # behavior of running a script from the system command line, where + # Python inserts the script's directory into sys.path + dname = str(fpath.parent) + + def get_cells() -> Any: + """generator for sequence of code blocks to run""" + if fpath.suffix == '.ipynb': + from nbformat import read + nb = read(fpath, as_version=4) + if not nb.cells: + return + for cell in nb.cells: + if cell.cell_type == 'code': + if getattr(cell, 'metadata', {}).get('language', '') == 'sql': + yield f'%%sql\n{cell.source}' + else: + yield cell.source + else: + yield fpath.read_text(encoding='utf-8') + + with prepended_to_syspath(dname): + try: + for cell in get_cells(): + result = self.run_cell(cell, silent=True, shell_futures=shell_futures) + if raise_exceptions: + result.raise_error() + elif not result.success: + break + except Exception: + if raise_exceptions: + raise + self.showtraceback() + warn('Unknown failure executing file: <%s>' % fpath) From 14b3fbf8ba6feb066140ba48f32e4ae71d2d9717 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 11:30:08 -0500 Subject: [PATCH 2/9] Fix %%sql cells in %run_shared magic --- singlestoredb/magics/run_shared.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/singlestoredb/magics/run_shared.py b/singlestoredb/magics/run_shared.py index bb41ec907..00d9a4bea 100644 --- a/singlestoredb/magics/run_shared.py +++ b/singlestoredb/magics/run_shared.py @@ -56,7 +56,7 @@ def run_shared(self, line: str, local_ns: Any = None) -> Any: # Run the downloaded file with preserve_keys(self.shell.user_ns, '__file__'): self.shell.user_ns['__file__'] = temp_file_path - self.shell.safe_execfile_ipy(temp_file_path, raise_exceptions=True) + self.safe_execfile_ipy(temp_file_path, raise_exceptions=True) def safe_execfile_ipy( self, From 43c4caf6c8ef711eae21d04c7eb9ddc4e0f9d6a5 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 11:34:58 -0500 Subject: [PATCH 3/9] Fix %%sql cells in %run_shared magic --- singlestoredb/magics/run_shared.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/singlestoredb/magics/run_shared.py b/singlestoredb/magics/run_shared.py index 00d9a4bea..4d8ba1302 100644 --- a/singlestoredb/magics/run_shared.py +++ b/singlestoredb/magics/run_shared.py @@ -113,7 +113,9 @@ def get_cells() -> Any: with prepended_to_syspath(dname): try: for cell in get_cells(): - result = self.run_cell(cell, silent=True, shell_futures=shell_futures) + result = self.shell.run_cell( + cell, silent=True, shell_futures=shell_futures, + ) if raise_exceptions: result.raise_error() elif not result.success: @@ -121,5 +123,5 @@ def get_cells() -> Any: except Exception: if raise_exceptions: raise - self.showtraceback() + self.shell.showtraceback() warn('Unknown failure executing file: <%s>' % fpath) From ca44e975a4cf1e065822f3c47a06bca8410bd40b Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 12:45:35 -0500 Subject: [PATCH 4/9] Add output redirection --- singlestoredb/magics/run_shared.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/singlestoredb/magics/run_shared.py b/singlestoredb/magics/run_shared.py index 4d8ba1302..ed1e87bf2 100644 --- a/singlestoredb/magics/run_shared.py +++ b/singlestoredb/magics/run_shared.py @@ -103,8 +103,13 @@ def get_cells() -> Any: return for cell in nb.cells: if cell.cell_type == 'code': + output_redirect = getattr( + cell, 'metadata', {}, + ).get('output_variable', '') + if output_redirect: + output_redirect = f' << {output_redirect}' if getattr(cell, 'metadata', {}).get('language', '') == 'sql': - yield f'%%sql\n{cell.source}' + yield f'%%sql{output_redirect}\n{cell.source}' else: yield cell.source else: From a9042f4cad4b978683bd71ec79913db2b1176de2 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 12:49:48 -0500 Subject: [PATCH 5/9] Add output redirection --- singlestoredb/magics/run_shared.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/singlestoredb/magics/run_shared.py b/singlestoredb/magics/run_shared.py index ed1e87bf2..62afe3bbc 100644 --- a/singlestoredb/magics/run_shared.py +++ b/singlestoredb/magics/run_shared.py @@ -105,9 +105,9 @@ def get_cells() -> Any: if cell.cell_type == 'code': output_redirect = getattr( cell, 'metadata', {}, - ).get('output_variable', '') + ).get('output_variable', '') or '' if output_redirect: - output_redirect = f' << {output_redirect}' + output_redirect = f' {output_redirect} <<' if getattr(cell, 'metadata', {}).get('language', '') == 'sql': yield f'%%sql{output_redirect}\n{cell.source}' else: From 58cad2dda0a88c58496ae8e2436e70a5e358d82a Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 12:51:23 -0500 Subject: [PATCH 6/9] Skip empty cells --- singlestoredb/magics/run_shared.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/singlestoredb/magics/run_shared.py b/singlestoredb/magics/run_shared.py index 62afe3bbc..e7850504a 100644 --- a/singlestoredb/magics/run_shared.py +++ b/singlestoredb/magics/run_shared.py @@ -103,6 +103,8 @@ def get_cells() -> Any: return for cell in nb.cells: if cell.cell_type == 'code': + if not cell.source.strip(): + continue output_redirect = getattr( cell, 'metadata', {}, ).get('output_variable', '') or '' From 12941e71808a1fbe7ac10a5f7d82bae6e278bd38 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 12:54:59 -0500 Subject: [PATCH 7/9] Make changes to personal magic too --- singlestoredb/magics/run_personal.py | 83 +++++++++++++++++++++++++++- 1 file changed, 82 insertions(+), 1 deletion(-) diff --git a/singlestoredb/magics/run_personal.py b/singlestoredb/magics/run_personal.py index a83e7b1a2..639366581 100644 --- a/singlestoredb/magics/run_personal.py +++ b/singlestoredb/magics/run_personal.py @@ -1,6 +1,8 @@ import os import tempfile +from pathlib import Path from typing import Any +from warnings import warn from IPython.core.interactiveshell import InteractiveShell from IPython.core.magic import line_magic @@ -8,6 +10,8 @@ from IPython.core.magic import magics_class from IPython.core.magic import needs_local_scope from IPython.core.magic import no_var_expand +from IPython.utils.contexts import preserve_keys +from IPython.utils.syspathcontext import prepended_to_syspath from jinja2 import Template @@ -53,4 +57,81 @@ def run_personal(self, line: str, local_ns: Any = None) -> Any: # Execute the SQL command self.shell.run_line_magic('sql', sql_command) # Run the downloaded file - self.shell.run_line_magic('run', f'"{temp_file_path}"') + with preserve_keys(self.shell.user_ns, '__file__'): + self.shell.user_ns['__file__'] = temp_file_path + self.safe_execfile_ipy(temp_file_path, raise_exceptions=True) + + def safe_execfile_ipy( + self, + fname: str, + shell_futures: bool = False, + raise_exceptions: bool = False, + ) -> None: + """Like safe_execfile, but for .ipy or .ipynb files with IPython syntax. + + Parameters + ---------- + fname : str + The name of the file to execute. The filename must have a + .ipy or .ipynb extension. + shell_futures : bool (False) + If True, the code will share future statements with the interactive + shell. It will both be affected by previous __future__ imports, and + any __future__ imports in the code will affect the shell. If False, + __future__ imports are not shared in either direction. + raise_exceptions : bool (False) + If True raise exceptions everywhere. Meant for testing. + """ + fpath = Path(fname).expanduser().resolve() + + # Make sure we can open the file + try: + with fpath.open('rb'): + pass + except Exception: + warn('Could not open file <%s> for safe execution.' % fpath) + return + + # Find things also in current directory. This is needed to mimic the + # behavior of running a script from the system command line, where + # Python inserts the script's directory into sys.path + dname = str(fpath.parent) + + def get_cells() -> Any: + """generator for sequence of code blocks to run""" + if fpath.suffix == '.ipynb': + from nbformat import read + nb = read(fpath, as_version=4) + if not nb.cells: + return + for cell in nb.cells: + if cell.cell_type == 'code': + if not cell.source.strip(): + continue + output_redirect = getattr( + cell, 'metadata', {}, + ).get('output_variable', '') or '' + if output_redirect: + output_redirect = f' {output_redirect} <<' + if getattr(cell, 'metadata', {}).get('language', '') == 'sql': + yield f'%%sql{output_redirect}\n{cell.source}' + else: + yield cell.source + else: + yield fpath.read_text(encoding='utf-8') + + with prepended_to_syspath(dname): + try: + for cell in get_cells(): + result = self.shell.run_cell( + cell, silent=True, shell_futures=shell_futures, + ) + if raise_exceptions: + result.raise_error() + elif not result.success: + break + except Exception: + if raise_exceptions: + raise + self.shell.showtraceback() + warn('Unknown failure executing file: <%s>' % fpath) From 09f4fdcdf3bee2f46a6d8b57bab8c867a26dd683 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Fri, 13 Jun 2025 13:13:42 -0500 Subject: [PATCH 8/9] Fix action name --- .github/workflows/code-check.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/code-check.yml b/.github/workflows/code-check.yml index f0f0ad9a8..dc19d8952 100644 --- a/.github/workflows/code-check.yml +++ b/.github/workflows/code-check.yml @@ -1,4 +1,4 @@ -name: Coverage tests +name: Code checks on: push: From 9cc8e9a447166b3895341ff75193bd384945c0e6 Mon Sep 17 00:00:00 2001 From: Kevin Smith Date: Mon, 14 Jul 2025 08:48:21 -0500 Subject: [PATCH 9/9] Only test for output redirect for SQL cells --- singlestoredb/magics/run_personal.py | 10 +++++----- singlestoredb/magics/run_shared.py | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/singlestoredb/magics/run_personal.py b/singlestoredb/magics/run_personal.py index 639366581..04847b223 100644 --- a/singlestoredb/magics/run_personal.py +++ b/singlestoredb/magics/run_personal.py @@ -108,12 +108,12 @@ def get_cells() -> Any: if cell.cell_type == 'code': if not cell.source.strip(): continue - output_redirect = getattr( - cell, 'metadata', {}, - ).get('output_variable', '') or '' - if output_redirect: - output_redirect = f' {output_redirect} <<' if getattr(cell, 'metadata', {}).get('language', '') == 'sql': + output_redirect = getattr( + cell, 'metadata', {}, + ).get('output_variable', '') or '' + if output_redirect: + output_redirect = f' {output_redirect} <<' yield f'%%sql{output_redirect}\n{cell.source}' else: yield cell.source diff --git a/singlestoredb/magics/run_shared.py b/singlestoredb/magics/run_shared.py index e7850504a..165a6de40 100644 --- a/singlestoredb/magics/run_shared.py +++ b/singlestoredb/magics/run_shared.py @@ -105,12 +105,12 @@ def get_cells() -> Any: if cell.cell_type == 'code': if not cell.source.strip(): continue - output_redirect = getattr( - cell, 'metadata', {}, - ).get('output_variable', '') or '' - if output_redirect: - output_redirect = f' {output_redirect} <<' if getattr(cell, 'metadata', {}).get('language', '') == 'sql': + output_redirect = getattr( + cell, 'metadata', {}, + ).get('output_variable', '') or '' + if output_redirect: + output_redirect = f' {output_redirect} <<' yield f'%%sql{output_redirect}\n{cell.source}' else: yield cell.source