Allow metadata for write_deltalake #587
Conversation
There was a problem hiding this comment.
LGTM, will leave it to @fvaleye and @wjones127 to do the final review and merge.
|
looks like python linter is also complaining about too many arguments. |
|
Not sure why the lambda_checkpoint_build failed, doesn't seem to be related to changes made here. LMK if I need to change anything else here, but doesn't seem to be required so I'll be leaving it as-is for now. |
There was a problem hiding this comment.
Thanks! Code changes look pretty clean, just needs a few updates to the documentation. 👍
Also this made me realize we'll also probably want AWS Glue and Hive metastore integration; otherwise giving the table a name isn't that useful 😕 . But I think it's worth doing that in a different PR.
| @@ -44,6 +44,9 @@ def write_deltalake( | |||
| partition_by: Optional[List[str]] = None, | |||
| filesystem: Optional[pa_fs.FileSystem] = None, | |||
| mode: Literal["error", "append", "overwrite", "ignore"] = "error", | |||
| name: Optional[str] = None, | |||
There was a problem hiding this comment.
Could we also add a note in the docstring that this function does not register this table in your data catalog? Users will have to either use the filesystem path to the table or manually register this table in the catalog. I'll create a follow up issue for us to handle that though.
There was a problem hiding this comment.
Added a brief note. Let me know if more explanation re: manual add is needed.
|
And don't worry about the checkpoint build, we've seen it be flaky recently and it doesn't seem like it could be related to these changes. |
|
Thank you @PadenZach! |
Description
Related Issue(s)
Should resolve #576
Documentation