| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
…ing bigquery/gcs and all its dependencies.
|
For reference, I based the approach on these: |
Sorry, something went wrong.
|
Really good reference on import hooks here https://github.com/0cjs/sedoc/blob/master/lang/python/importers.md |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks Victor for diving into the weeds of Python import system and find this solution for b/137572393! :)
Our users will really appreciate.
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM (very cool that you found a solution here!!!)
fyi: we may have a tiny merge conflict in sitecustomize.py due to my pending PR.
Sorry, something went wrong.
| # since we call kaggle_gcp to load the module. | ||
| if self._is_called_from_kaggle_gcp(): | ||
| return None | ||
| spec = importlib.machinery.ModuleSpec(fullname, GcpModuleLoader()) |
There was a problem hiding this comment.
nit: why not just return importlib.machinery.ModuleSpec(fullname, GcpModuleLoader()), is it more Pythonic?
Sorry, something went wrong.
There was a problem hiding this comment.
Done.
Was leftover from when I was logging the spec :)
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM great!
Sorry, something went wrong.
|
Tests pass, and I've checked that starting python in the container indeed doesn't load all of google.cloud.* like it used to. Now I'm building the container locally to test on a Kernel just to make sure everything runs correctly. |
Sorry, something went wrong.
|
Tested it end to end locally, and looks good! Would still be good to test more thoroughly when we have a staging image to release by running with existing bigquery kernels in prod. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
…ing bigquery/gcs and all its dependencies.
I realize this change is pretty confusing and took me a while to get to a working state, so feel free to comment, ask for clarifications. I'll try to explain what it's doing but let me know if it's not clear.
This tries to use import hooks (as suggested by Vincent, thanks!) to do the monkeypatching at module load time (versus when sitecustomize is loaded prior to user code execution). This resolves the issue of having tons of modules (google.cloud.* and all its deps) loaded for Kernels that may never use bigquery/gcs. The strategy is: setup an import hook that checks when one of our special libraries is being loaded and instead of loading the one in the module path, load our monkeypatched one.
The issue is, our monkeypatching uses kaggle_gcp which itself loads google.cloud.bigquery (it has to, since we do the monkeypatching by subclassing bigquery), which causes a circular dependency/recursive imports. We get around this by adding a check in the module finder hook to return the actual bigquery library if the call stack originates from kaggle_gcp, thus avoiding the circular dependency. We still have the issue that if the user (or test code) calls import kaggle_gcp before from google.cloud import bigquery, we will load the true bigquery library during the kaggle_gcp load and never end up monkeypatching it since google.cloud.bigquery will already have been loaded. To solve this, I add a init() to the end of kaggle_gcp so if ever kaggle_gcp is imported, then the monkeypatching will guaranteed to be done even after the true bigquery library has been loaded.