livebook-dev / kino_db

Database integrations for Livebook
Apache License 2.0
40 stars 16 forks source link

Add support for req_snowflake #31

Closed joshuataylor closed 1 year ago

joshuataylor commented 2 years ago

This is an initial PR to get feedback around adding req_snowflake support to kino_db. image

Cool things about this:

  1. We use table to only get the results we care about. It won't download all the chunks from Snowflake right away, it will only download the relevant chunk you are on.
  2. It will cache your token between requests, so it does not need to re-authenticate. If it can't use the token it will reauthenticate anyway (Snowflake authentication is a second or two, but IMHO it's best to reduce this, as it's a notable difference)

There are some weird usability issues:

  1. The box for database, schema, etc is long, and with no scrollbar this is confusing.
  2. I want to add additional parameters that a user might use (session_parameters from https://docs.snowflake.com/en/sql-reference/parameters.html for example), but not sure how to do this in a sane way that doesn't involve extending the required params?

The following things need to happen before being merged:

[ ] Add rustler precompiled support to snowflake_arrow [ ] Release a version of snowflake_arrow to hex.pm with above [ ] Release a version of req_snowflake on hex.pm - this will be done once the repo has been setup by @josevalim (if you still want req_snowflake here, no dramas either way)

josevalim commented 2 years ago

Absolute beautiful, thank you for the PR! :heart:

The box for database, schema, etc is long, and with no scrollbar this is confusing.

Our smart cells have static sizes but you should be able to break the input into several rows, like in the other connection cells.

I want to add additional parameters that a user might use (session_parameters from https://docs.snowflake.com/en/sql-reference/parameters.html for example), but not sure how to do this in a sane way that doesn't involve extending the required params?

Maybe a text area where each parameter is written in its own line as KEY=VALUE?

[ ] Add rustler precompiled support to snowflake_arrow

Ideally I would love if we could use Explorer instead. The rationale is that, once we add Explorer integration, Explorer will provide the best APIs within Livebook to further query, manipulate and transform data, and if we can get the data into Explorer upfront, then it will actually be the most efficient route.

I will be glad to work together on whatever is necessary to make Explorer+Snowflake as efficient as it can be. :) What are your thoughts? :)

joshuataylor commented 2 years ago

Agreed with all your points.

The awesome thing is that once we get the SF+Arrow integration right, we can use the same logic for BigQuery.

nzscripps commented 1 year ago

What is the current status of this PR? I, too, work at a company that uses Snowflake, and am hoping to use Livebook and something like this to introduce Elixir to the organization. Is there any help I could provide to help move this forward?

josevalim commented 1 year ago

@nzscripps i am working on integrating ADBC with Explorer, which does have Snowflake support, which should hopefully be quite efficient. So this will happen through other channels but it will take a few months still.

josevalim commented 1 year ago

I will close this for now. Snowflake support will arrive via Explorer+ADBC. Thanks!