alamb commented on code in PR #5025:
URL: https://github.com/apache/arrow-datafusion/pull/5025#discussion_r1085658317


##########
datafusion/core/src/execution/options.rs:
##########
@@ -377,9 +340,112 @@ impl<'a> NdJsonReadOptions<'a> {
         self.schema = Some(schema);
         self
     }
+}
 
+#[async_trait]
+///

Review Comment:
   Can you please fill in this docstring?



##########
datafusion/core/src/execution/options.rs:
##########
@@ -377,9 +340,112 @@ impl<'a> NdJsonReadOptions<'a> {
         self.schema = Some(schema);
         self
     }
+}
 
+#[async_trait]
+///
+pub trait ReadOptions<'a> {
     /// Helper to convert these user facing options to `ListingTable` options
-    pub fn to_listing_options(&self, target_partitions: usize) -> 
ListingOptions {
+    fn to_listing_options(&self, target_partitions: usize) -> ListingOptions;

Review Comment:
   ```suggestion
       fn to_listing_options(&self, config: &SessionConfig) -> ListingOptions;
   ```
   
   While I realize that the `target_partitions` is the only thing actually used 
at the moment, since this is part of the public API I think providing 
`&SessionConfig` would be more future proof (aka avoid changes to this trait 
over time)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to