-
Notifications
You must be signed in to change notification settings - Fork 32
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
Rapids on snowflake deployment #493
Conversation
Can't seem to find a fix for the failure: Looks like it is sort of a known issue sphinx-doc/sphinx#3175 but the suggested fix, I found relies on this sphinx syntax and when trying this, it doesn't render as expected. Any ideas?
|
@jacobtomlinson I think this is ready for a first review. That being said while checking if the volume mount instructions where working correctly I realized that the mount won't work unless we have We have two options here.
FROM rapidsai/notebooks:25.02a-cuda11.8-py3.11-amd64
RUN pip install "snowflake-snowpark-python[pandas]" snowflake-connector-python
USER rapids
WORKDIR /home/rapids/workspace TODO
|
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 struggled with some permissions error when following these instructions. Could you take a look and see wha I'm doing wrong?
Also to answer your question I wouldn't set the working directory I would just create an empty directory.
We don't need to creater the directory ahead of time. Setting mountPath: /home/rapids/notebooks/workspace
works for me.
source/platforms/snowflake.md
Outdated
Create a Dockerfile as follow: | ||
|
||
```Dockerfile | ||
FROM rapidsai/notebooks:25.02a-cuda11.8-py3.11-amd64 |
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.
This will go stale. We usually use templating to ensure it always shows the correct RAPIDS image. The requirements to use Python 3.11 may add a little complexity here.
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.
Yes, the requirement of python 3.11 and explicit amd64 platform will make the templating a bit more complicated. I'll take a look into it
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 managed to test the instructions end-to-end and I managed to get a working notebook instance and ran some cudf
code 🎉.
I found a few more things that need tweaking though and left a few more comments.
source/platforms/snowflake.md
Outdated
ALTER COMPUTE POOL CONTAINER_HOL_POOL STOP ALL; | ||
ALTER COMPUTE POOL CONTAINER_HOL_POOL SUSPEND; | ||
|
||
DROP SERVICE CONTAINER_HOL_DB.PUBLIC.RAPIDS_SNOWPARK_SERVICE; |
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.
This line didn't work for me, it just said the service didn't exist, but the rest of the commands did so I think I cleaned up successfully.
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.
Interesting, this is as suggested by snowflake. Did your service had a different name or something? I'll give this another try after again soon and check.
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.
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 can replicate it,I guess this is a bug on their snowflake quickstart.
I had a good amount of trouble launching things again, I think there was some cleanup and role mix up instructions that didn't remove everything. But hopefully that's sorted. Yet TODO:
|
a5c7471
to
544eaf3
Compare
This should be ready now for final review, no TODOs left. Looks like I mess up something when rebasing, I think I fix it and it should be fine. Do we have squashing here when merging or do I need to cleanup the commits? |
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 ran through end-to-end and everything works great for me. I added a couple of comments but generally this is great.
source/platforms/snowflake.md
Outdated
ALTER COMPUTE POOL CONTAINER_HOL_POOL STOP ALL; | ||
ALTER COMPUTE POOL CONTAINER_HOL_POOL SUSPEND; | ||
|
||
DROP SERVICE CONTAINER_HOL_DB.PUBLIC.RAPIDS_SNOWPARK_SERVICE; |
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.
Co-authored-by: Jacob Tomlinson <[email protected]>
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.
This looks great. I think we are at a point where we can merge this and follow up in additional PRs.
Closes the deployment part of #419
For the "Show reading data from Snowflake into a cudf dataframe" , I created this issue for tracking purposes #494
This is WIP but it's almost there for review, need a few more sections with screenshots.TODO:
$
. Any suggestions @jacobtomlinson ?