Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces Jupyter Notebook Extras to automatically load interactive conveniences like explore_dataframe(), %dpip, and %%sparksql when importing the package inside an IPython kernel. It includes configuration options to opt out of these extras, updates dependencies, and adds comprehensive tests. The review feedback highlights a potential memory leak in _ipython.py due to strong references to the IPython shell in _SHELL_STATES, recommending the use of weakref.WeakKeyDictionary. Additionally, a minor grammatical redundancy was pointed out in the README.md documentation.
Automatically initialize interactive notebook extras when importing google.cloud.managed_spark_connect inside an IPython kernel: - Inject colabsqlviz's explore_dataframe() into IPython user_ns - Load the %dpip line magic extension (google.cloud.managed_spark_magics) - Load the %%sparksql cell magic extension (sparksql_magic) - Add google-colabsqlviz>=0.3.0 and sparksql-magic>=0.0.3 to dependencies Add opt-out and runtime configuration controls: - Environment variable: MANAGED_SPARK_CONNECT_ENABLE_EXTRAS=false - IPython traitlet: ManagedSparkConnect.enable_extras = False (supports both persistent file-based config and runtime toggling via %config, tracking and only undoing changes that managed_spark_connect itself performed).
95f3ebd to
62a606f
Compare
| # Imported directly by managed_spark_connect._ipython and | ||
| # managed_spark_magics; previously these only arrived transitively via | ||
| # google-colabsqlviz and sparksql-magic. | ||
| "ipython>=8.0", |
There was a problem hiding this comment.
I feel like we may want to make all of these optional - we have customers that do not use notebooks w/ Spark Connect, for them these dependencies are not necessary.
There was a problem hiding this comment.
The goal is to make the out-of-the box experience as smooth as possible for notebook users.
I agree that we shouldn't introduce performance regressions for non-notebook users. The impact of these changes for non-notebook users are:
- 11M of extra wheel downloads
- 0 runtime cost (short-circuits when get_ipython is not importable or returns None)
I'm sympathetic to wanting to avoid the extra wheels, but there is no perfect option:
pip install google-cloud-spark-connect[notebook]is extra wordy, and agents/humans might fail to add the extra- It's not technically possible to make
pip install google-cloud-spark-connect[no-notebook]remove the dependencies - Optionally importing them only if available means you need to do
pip install google-cloud-spark-connect sparksql-magic google-colabsqlviz [...this list will grow over time...]which is like (1) but worse
On balance, we're saying that the extra wheel download cost is better any of the alternatives 1-3
There was a problem hiding this comment.
I think that it's beyond wheel download, it can cause actual incompatibilities in env, and agents does not need this as well?
I think that to make it practically useful we would need to split it out in the and even exclude pyspark by default:
pip install google-cloud-spark-connect[notebook] pyspark-client
This is still one line step that users/agents will copy from docs, but it allow us to support all the use cases.
| except Exception: | ||
| pass | ||
|
|
||
| _init_extras() |
There was a problem hiding this comment.
Did we measure latency of this call?
There was a problem hiding this comment.
I can, but it's just loading a couple python modules so I don't imagine it will be more than a few ms.
There was a problem hiding this comment.
We had 20s regression with some AI client libs - I think worth to check this ahead of time.
There was a problem hiding this comment.
Importing is slower than I thought, but still only ~35ms on my laptop. ~27ms of that is importing anywidget. Assuming they are going to run any Spark operations I think this is negligible.
$ t() { MANAGED_SPARK_CONNECT_ENABLE_EXTRAS=0 ipython -c 'import os, time, google.cloud.managed_spark_connect._ipython as m
m._SHELL_STATES.clear(); os.environ["MANAGED_SPARK_CONNECT_ENABLE_EXTRAS"] = "1"
t0 = time.perf_counter(); m._init_extras(); print(f"{(time.perf_counter() - t0) * 1e3:.0f}ms")' 2>/dev/null | tail -1; }
evict() { if [ "$(uname)" = Darwin ]; then sudo purge; else python -c 'import os, site; [os.posix_fadvise(fd := os.open(os.path.join(d, f), os.O_RDONLY), 0, 0, os.POSIX_FADV_DONTNEED) or os.close(fd) for p in site.getsitepackages() for d, _, fs in os.walk(p) for f in fs]'; fi; }
for i in 1 2 3; do echo "hot: $(t) cold page cache: $(evict; t) no .pyc: $(PYTHONPYCACHEPREFIX=$(mktemp -d) t)"; done
MANAGED_SPARK_CONNECT_ENABLE_EXTRAS=0 python -X importtime -m IPython -c 'import os, sys, google.cloud.managed_spark_connect._ipython as m
m._SHELL_STATES.clear(); os.environ["MANAGED_SPARK_CONNECT_ENABLE_EXTRAS"] = "1"; print("MARK", file=sys.stderr, flush=True); m._init_extras()' 2>&1 >/dev/null | awk -F'|' 'f && $2 >= 5000 {l[n++] = sprintf("%6.1fms %s", $2 / 1000, $3)} /^MARK$/ {f = 1} END {print "cumulative import time during _init_extras (>=5ms):"; while (n) print l[--n]}'
hot: 47ms cold page cache: 56ms no .pyc: 98ms
hot: 36ms cold page cache: 45ms no .pyc: 102ms
hot: 35ms cold page cache: 47ms no .pyc: 102ms
cumulative import time during _init_extras (>=5ms):
33.6ms google.colabsqlviz.explore_dataframe
31.6ms google.colabsqlviz.interactive_viz
27.6ms anywidget
20.2ms anywidget.widget
19.8ms ipywidgets
19.4ms ipywidgets.widgets
6.8ms anywidget._traits
6.7ms anywidget._descriptor
6.0ms anywidget._file_contents
5.8ms psygnal
Automatically initialize interactive notebook extras when importing google.cloud.managed_spark_connect inside an IPython kernel:
Add opt-out and runtime configuration controls:
Dependency & Footprint Justification
The new hard dependencies added by this PR (
google-colabsqlviz,sparksql-magic) add just a few MiB of wheel downloads. In more detail:All core data dependencies (
pyspark,pandas>=2.0.0,pyarrow>=10.0.1,protobuf>=4.24.0,packaging>=20.0) are already satisfied bypyspark[connect]andgoogle-api-core. Installing both libraries causes 0 upgrades or downgrades to existing packages.sparksql-magicHas Zero Extra Transitive Cost:sparksql-magic(4.2 KiBwheel,6.9 KiBextracted,14.4 KiBwith.pyc) only depends onpysparkandipython—a strict subset ofgoogle-colabsqlvizandipykernel. Adding it alongsidegoogle-colabsqlvizadds 0 additional transitive dependencies.In local notebook kernels (e.g., VS Code or Jupyter, which require
ipykernel), the entireipythonstack is already present. Adding both libraries requires downloading just 3.59 MiB of wheels (~1.7% of the existing install size dominated bypyspark@ ~435 MiB wheel andpyarrow@ ~130 MiB):.pyc, e.g.uv).pyc(pip)pip)ipykernel+ipywidgets)google-colabsqlviz,sparksql-magic,anywidget,psygnal)ipykernelonly)ipywidgets,jupyterlab-widgets,widgetsnbextension)ipythonstack)(Note: Pure Python code across all 7 packages added in the minimal VS Code kernel scenario is < 1 MiB. The remaining extracted space is static frontend JS bundles/source maps in
widgetsnbextensionandanywidget[~11.5 MiB],psygnal's compiled mypyc.sobinary [~1.3 MiB], andpip's.pycbytecode cache duplicating embedded JS strings incolabsqlviz[~1.0 MiB].)