-
Notifications
You must be signed in to change notification settings - Fork 2
feat(BUG-1150): adds examples notebooks for Unity Catalog access #81
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
base: main
Are you sure you want to change the base?
Conversation
Found 2 changed notebooks. Review the changes at https://app.gitnotebooks.com/wherobots/wherobots-examples/pull/81 |
3516cca
to
0bb08b3
Compare
still TODO, we should link to docs from instructions that reference creating a foriegn catalog
0bb08b3
to
9bdb7ce
Compare
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.
@kadolor im not too familiar with notebook authoring guidelines, the use of "we" and "you" in this is a bit strange to me but otherwise this seems fine to me?
"# Before You Begin\n", | ||
"This example assumes you have already created a Delta-format foreign catalog connection to Databricks Unity Catalog, which contains a Delta table to operate on. This Delta table must contain a geometry column with values encoded in WKT format.\n", | ||
"\n", | ||
"If you plan to write the data to an Iceberg table in Unity Catalog, you will need another Iceberg-format foreign catalog connection.\n", |
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 don't quite understand what ultimately is providing here but I may be lacking context
" \"properties\": {},\n", | ||
" \"geometry\": {\n", | ||
" \"type\": \"Polygon\", \n", | ||
" \"coordinates\": [\n", |
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.
Where is this AOI?
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.
@aminianp I'm not sure it exists--I think this notebook is positioned as: 'You already connected your notebook and here's an example of an ETL procedure'.
I think there needs to be further contextualizing at the beginning about how customers should read this notebook, that is, as general guidance through the ETL process depending on the type of Table you're loading and writing to.
@kcheng486 @RoboDonut If lines 1-4 were filled in with valid values by someone running this notebook, my assumption is that this notebook would still error out, is that correct?
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.
sorry I don't quite follow why this would error out? presumably it would just give nothing if no data was found within the AOI? that said the polygon seems like it only spans a very small area in the Gulf of Guinea?
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.
@kadolor i think the point you're making is that if the data in your foreign catalog doesn't happen to line up with the aoi, then you'll get an empty data set when you apply the filter. am i understanding you correctly?
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.
Left some nit comments here and there; otherwise LGTM.
Co-authored-by: Kelly-Ann Dolor <[email protected]>
Co-authored-by: Kelly-Ann Dolor <[email protected]>
Co-authored-by: Kelly-Ann Dolor <[email protected]>
Co-authored-by: Kelly-Ann Dolor <[email protected]>
Co-authored-by: Kelly-Ann Dolor <[email protected]>
Co-authored-by: Kelly-Ann Dolor <[email protected]>
Co-authored-by: Kelly-Ann Dolor <[email protected]>
Co-authored-by: Kelly-Ann Dolor <[email protected]>
still TODO, we should link to docs from instructions that reference creating a foriegn catalog