[GitHub] [arrow-ballista] avantgardnerio commented on a diff in pull request #501: Automatically register tables if env var specified

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

avantgardnerio commented on code in PR #501:
URL: https://github.com/apache/arrow-ballista/pull/501#discussion_r1018208929


##########
ballista/client/src/context.rs:
##########
@@ -137,13 +139,17 @@ impl BallistaContext {
     pub async fn standalone(
         config: &BallistaConfig,
         concurrent_tasks: usize,
+        table_factories: HashMap<String, Arc<dyn TableProviderFactory>>,

Review Comment:
   > can the table_factories be determined by the ballista config?
   
   Unfortunately, I don't think so:
   
   ```
   #[derive(Debug, Clone, PartialEq, Eq)]
   pub struct BallistaConfig {
   ```
   
   I don't think it is logical for `TableProviderFactories` to implement `PartialEq` or `Eq`, and without making another DataFusion PR, they don't impl `Debug` at present either. Everything else in the `BallistaConfig` is a key-value pair, and extending it to include stateful runtime objects seems like a questionable choice.
   
   I can however swap out the Map for an `Arc<dyn SessionBuilder>`, but then `client` needs to have a new dependency on `core`.



-- 
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