-
Notifications
You must be signed in to change notification settings - Fork 972
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use kagglehub inside Kaggle Notebook for tensorflow_hub #1342
Conversation
Currently, if you try using tensorflow_hub with a kaggle.com URL inside a Kaggle notebook but the model isn't attached you would get an error message telling you to attach the model. With this change, we are calling `kagglehub` instead and will attach the model for the user if it is missing. http://b/311184735
|
||
def _is_on_kaggle_notebook(): | ||
return os.getenv("KAGGLE_CONTAINER_NAME") != None | ||
return os.getenv("KAGGLE_KERNEL_RUN_TYPE") != None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using the same environment variable than kagglehub
. Both are set so this is a NoOp: https://github.com/Kaggle/kagglehub/blob/c1b6e00226ca952b1d93d05089980dfe233f0163/src/kagglehub/kaggle_cache_resolver.py#L29C1-L29C1
"mountSlug": mount_slug, | ||
}, | ||
}), "utf-8")) | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: start with this case to reduce tabs:
if not self.path.endswith(...):
self.send_response(404)
...
content_length = int(self.headers["Content-Length"])
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to keep it this way because we may add new elif
cases for other paths and we would want to fail only in the else
at the bottom.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the image in a Kaggle notebook and it works.
The test failure is unrelated. See b/316967430
Use kagglehub inside Kaggle Notebook for tensorflow_hub
Currently, if you try using tensorflow_hub with a kaggle.com URL inside
a Kaggle notebook but the model isn't attached you would get an error
message telling you to attach the model.
With this change, we are calling
kagglehub
instead and will attach themodel for the user if it is missing.
http://b/311184735