feat: add restore command in python binding #1529
Conversation
loleek
requested review from
wjones127,
fvaleye,
rtyler,
roeap,
houqp,
xianwill and
mosyp
as code owners
github-actions
bot
added
binding/python
python/src/lib.rs
Outdated
| if let Some(version) = version_to_restore { | ||
| cmd = cmd.with_version_to_restore(version) | ||
| } | ||
| if let Some(ds) = datetime_to_restore { |
There was a problem hiding this comment.
Since both of these parameters are optional there is a potential for a user to inadvertently provide both arguments and that would result in two restore "commands" being passed into the builder, and the datetime one is the one which will win in that case.
I think it might be useful to raise an error here to indicate to the user that they should not provide both parameters, or issue a warning that the version_to_restore will be ignored.
There was a problem hiding this comment.
This check has been done here.parameter check. Should I do double check?
There was a problem hiding this comment.
I think the check is good. But TBH, for the Python interface, I think we should combine the parameters into one. See my other comment.
python/src/lib.rs
Outdated
| if let Some(version) = version_to_restore { | ||
| cmd = cmd.with_version_to_restore(version) | ||
| } | ||
| if let Some(ds) = datetime_to_restore { |
There was a problem hiding this comment.
I think the check is good. But TBH, for the Python interface, I think we should combine the parameters into one. See my other comment.
# Description This is a implementation of the Restore Command for python binding. # Related Issue(s) delta-io#837 --------- Co-authored-by: Will Jones <willjones127@gmail.com>
Description
This is a implementation of the Restore Command for python binding.
Related Issue(s)
#837