[GitHub] [arrow-datafusion-python] isidentical commented on a diff in pull request #76: allow config options to be passed to context

[GitBox <gi...@apache.org>]

isidentical commented on code in PR #76:
URL: https://github.com/apache/arrow-datafusion-python/pull/76#discussion_r1026635637


##########
src/context.rs:
##########
@@ -75,9 +79,23 @@ impl PySessionContext {
         repartition_windows: bool,
         parquet_pruning: bool,
         target_partitions: Option<usize>,
-        // TODO: config_options
+        config_options: Option<HashMap<String, String>>,
     ) -> Self {
-        let cfg = SessionConfig::new()
+        let mut options = ConfigOptions::new();

Review Comment:
   Suggestion: we probably should inherit everything from the current environment, and the passed configurations should override them.
   
   ```suggestion
           let mut options = ConfigOptions::from_env();
   ```



##########
src/context.rs:
##########
@@ -75,9 +79,23 @@ impl PySessionContext {
         repartition_windows: bool,
         parquet_pruning: bool,
         target_partitions: Option<usize>,
-        // TODO: config_options
+        config_options: Option<HashMap<String, String>>,

Review Comment:
   Does this mean we have to pass the stringized versions of rust types as config values? E.g. `{"batch_size": "4096"}` (instead of literal `4096`, or `"true"` instead of `True`). I guess it might be an improvement later (just to keep in mind).



##########
src/context.rs:
##########
@@ -86,6 +104,9 @@ impl PySessionContext {
             .with_repartition_windows(repartition_windows)
             .with_parquet_pruning(parquet_pruning);
 
+        // TODO we should add a `with_config_options` to `SessionConfig`

Review Comment:
   👍🏻 



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org