-
Notifications
You must be signed in to change notification settings - Fork 83
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
Preparing for deltalake v0.17 #162
Conversation
5b9d1cb
to
a303d18
Compare
a303d18
to
0f259a2
Compare
I am building other branches off of this one to help do integration testing and qualification of the next |
@@ -754,7 +754,7 @@ impl IngestProcessor { | |||
let dlq = dead_letter_queue_from_options(&opts).await?; | |||
let transformer = Transformer::from_transforms(&opts.transforms)?; | |||
let table = delta_helpers::load_table(table_uri, HashMap::new()).await?; | |||
let coercion_tree = coercions::create_coercion_tree(&table.get_metadata()?.schema); | |||
let coercion_tree = coercions::create_coercion_tree(table.schema().unwrap()); |
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.
Why switch from ?
to unwrap
? While this shouldn't really be a candidate for panicking, it is a possibility. Is that what we want?
@@ -947,11 +947,11 @@ impl IngestProcessor { | |||
|
|||
if self | |||
.delta_writer | |||
.update_schema(self.table.get_metadata()?)? | |||
.update_schema(self.table.state.delta_metadata().unwrap())? |
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.
Same as above.
{ | ||
info!("Table schema has been updated"); | ||
// Update the coercion tree to reflect the new schema | ||
let coercion_tree = coercions::create_coercion_tree(&self.table.get_metadata()?.schema); | ||
let coercion_tree = coercions::create_coercion_tree(self.table.schema().unwrap()); |
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.
Same as above.
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.
Looks good. I see that the delta crate has changed some things from results to options. That explains the unwrap
calls replacing the ?
operations. We should consider addressing it at some point.
Lots of refactorings necessary, the dynamodb lock mechanism will be changing too!