AnuragRaut08 opened a new pull request, #2395:
URL: https://github.com/apache/datafusion-ballista/pull/2395

   # Which issue does this PR close?
   
   Closes #2371.
   
   # Rationale for this change
   
   `BallistaConfig::entries()` currently emits every registered configuration 
key with its default value, even when the user has not explicitly configured 
that key.
   
   These defaults come from the client's version of the configuration registry. 
When a client and scheduler run different Ballista versions, the client can 
therefore send stale defaults that override the scheduler's current defaults.
   
   For example, a Python client built against an older Ballista version can 
send:
   
   ```text
   ballista.planner.adaptive.enabled = false
   ```
   
   during session creation even though the user never configured this setting. 
A newer scheduler whose default is `true` receives this as an explicitly 
provided value and applies `false`, silently changing the planner behaviour.
   
   This means that a configuration default changing between releases can be 
unintentionally overridden by an older client, without any explicit user 
configuration.
   
   # What changes are included in this PR?
   
   ### `ballista/core/src/config.rs`
   
   `BallistaConfig::entries()` now only emits values that are present in 
`self.settings`, meaning values that were explicitly configured by the user.
   
   Previously, the method used the registered default value when a key was 
absent from `self.settings`. It now returns `None` for unset keys.
   
   The receiving side already skips `None`-valued configuration entries, so 
unset settings are left to the scheduler/executor to resolve using their own 
current defaults.
   
   This also preserves the existing behaviour for local configuration reads. 
Methods such as `get_bool_setting` and `get_usize_setting` continue to fall 
back to `CONFIG_ENTRIES` when a setting has not been explicitly configured.
   
   Two Rust unit tests were added:
   
   * `default_ballista_config_emits_no_values_over_wire` — verifies that a 
default `BallistaConfig` does not emit concrete values for unset configuration 
keys.
   * `explicitly_set_key_is_present_in_entries` — verifies that explicitly 
configured values are still included and forwarded correctly.
   
   ### `python/python/tests/test_context.py`
   
   A Python integration regression test was added:
   
   * `test_unset_ballista_config_does_not_override_scheduler_defaults` — starts 
a test cluster with no client-side `cluster_config`, queries 
`information_schema.df_settings` for `ballista.planner.adaptive.enabled`, and 
verifies that the scheduler's own default (`true`) is preserved.
   
   This covers the cross-version behaviour described in #2371.
   
   # Are there any user-facing changes?
   
   Yes. This is a behavioural fix to configuration propagation.
   
   When a client has not explicitly configured a Ballista setting, the setting 
is no longer sent with the client's potentially stale default. The 
scheduler/executor can therefore apply its own current default.
   
   In particular, this prevents an older Python client from unintentionally 
overriding a newer scheduler's `ballista.planner.adaptive.enabled` default.
   
   Explicitly configured values continue to be forwarded unchanged, so 
user-specified configuration retains the same behaviour.
   


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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to