clean up duplicate information in FileOpener trait#17956
clean up duplicate information in FileOpener trait#17956adriangb merged 10 commits intoapache:mainfrom
Conversation
d649a43 to
d5a0cd9
Compare
d5a0cd9 to
3a5216a
Compare
xudong963
left a comment
There was a problem hiding this comment.
Good cleaning, thank you.
I saw there are some API changes, so added the api-change label.
|
Do you think it's worth adding this to the upgrade guide? I think it's quite straightforward and will impact few people. So maybe not worth the noise? |
| &self, | ||
| _partition_index: usize, | ||
| file_meta: FileMeta, | ||
| file: PartitionedFile, |
There was a problem hiding this comment.
1 nit: we probably can name it as partition_file to be the same format as other function params
| file_meta: FileMeta, | ||
| _file: PartitionedFile, | ||
| ) -> Result<FileOpenFuture> { | ||
| fn open(&self, file: PartitionedFile) -> Result<FileOpenFuture> { |
| &self, | ||
| partition_index: usize, | ||
| file_meta: FileMeta, | ||
| file: PartitionedFile, |
|
@comphead I've renamed all uses to |
|
Hmm I tried to put this in the queue but it seems to have merged immediately? I think it was ready but would have liked to see CI run. Not sure if it was a glitch in the GitHub UI or what. Anwyay I'll fix any issues in |
Yeah, we are working on the merge-queue but it doesn't actually re-run CI yet -- see reason here: #6880 (comment) |
| file_meta: FileMeta, | ||
| _file: PartitionedFile, | ||
| ) -> Result<FileOpenFuture> { | ||
| fn open(&self, partitioned_file: PartitionedFile) -> Result<FileOpenFuture> { |
There was a problem hiding this comment.
this is a nice improvement I agree
* clean up duplicate information in FileOpener trait * remove unused struct * remove more * rename paramter * more renames * minimize diff * fix renames * fix renames * more fixes
Ever since #16014 we've been passing duplicate information in.
FileMetais created entirely fromPartitionedFile. This simplifies APIs and removes an unnecessary struct.