Conversation
… add a test to bigquery magic for when spanner-graph-notebook is missing.
This is required for graph visualization.
…python runtime 3.12 instead of 3.8, as spanner-graph-notebook does not support runtime version 3.8.
…raph server is already running, due to another graph query having run previously.
| result = result.to_dataframe(**dataframe_kwargs) | ||
|
|
||
| if args.graph and _supports_graph_widget(result): | ||
| _add_graph_widget(result) |
There was a problem hiding this comment.
Should we do a warning if they asked for --graph but the results didn't support it?
There was a problem hiding this comment.
Long-term, I view --graph as a temporary flag to avoid breaking the standard code path while the graph implementation is not stable yet. Eventually, I would like to see the flag go away, and the graph visualization just be automatic in response to running a graph query.
So, I don't think such a warning is warranted right now.
bigquery_magics/graph_server.py
Outdated
| self.handle_post_query() | ||
|
|
||
|
|
||
| atexit.register(GraphServer.stop_server) |
There was a problem hiding this comment.
I'm not familiar with atexit, what does it do and why do we need it? Might be worth adding a few comments.
There was a problem hiding this comment.
I think the code is pretty much self-explanatory. It's necessary to prevent unit tests from hanging, due to the server background thread still running at the end of all of the tests.
| GraphServer._server = None | ||
|
|
||
|
|
||
| class GraphServerHandler(http.server.SimpleHTTPRequestHandler): |
There was a problem hiding this comment.
I'd love a few docstrings on these methods explaining their purpose. I can make some guesses based on the method names, but they are pretty generic names, so it's hard to interpret the intention.
| from google.cloud.spanner_v1.types import StructType, Type, TypeCode | ||
| import networkx | ||
| from spanner_graphs.conversion import ( | ||
| columns_to_native_numpy, |
There was a problem hiding this comment.
As of https://github.com/cloudspannerecosystem/spanner-graph-notebook/blob/cf18014e5af6309bfee537d070278aaf76cc5d1b/spanner_graphs/conversion.py this method no longer exists.
…anges to that repository broke our use of is.
feat: Support visualization of graph queries by adding the --graph argument.