Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ergonomic way to setup/configure SessionContextExt #1096

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

milenkovicm
Copy link
Contributor

@milenkovicm milenkovicm commented Oct 22, 2024

Which issue does this PR close?

Closes #1092.

Rationale for this change

This change provides ergonomic way to configure SessionContextExt, similar to DataFusion context setup from using provided SessionStore.

What changes are included in this PR?

This change provides two new methods in SessionContextExt async fn standalone_with_state(state: SessionState) and async fn remote_with_state(url: &str,state: SessionState)
Which accepts pre configured SessionState as parameter. SessionState should be configured in the same way like when SessionContext is configured in DataFusion.

let state = SessionStateBuilder::new().with_default_features().build();
let ctx: SessionContext = SessionContext::remote_with_state(&url, state).await?;

This change also exposes a BallistaSessionConfigExt which provides method to configure ballista specific settings like, BallistaConfiguration, codecs or even QueryPlanner.

use ballista_client::extension::BallistaSessionConfigExt;

let session_config = SessionConfig::new_with_ballista()
    .with_information_schema(true)
    .set_str(BALLISTA_JOB_NAME, "Super Cool Ballista App");

let state = SessionStateBuilder::new()
    .with_default_features()
    .with_config(session_config)
    .build();

let ctx: SessionContext = SessionContext::remote_with_state(&url, state).await?;

LogicalExtensionCodec and PhysicalExtensionCodec can be changed as well:

use ballista_client::extension::BallistaSessionConfigExt;
let logical_codec = Arc::new(BadLogicalCodec::default());
let physical_codec = Arc::new(MockPhysicalCodec::default());
let session_config = SessionConfig::new_with_ballista()
    .with_information_schema(true)
    .with_ballista_physical_extension_codec(physical_codec.clone())
    .with_ballista_logical_extension_codec(logical_codec.clone())
    ;
let state = SessionStateBuilder::new()
    .with_default_features()
    .with_config(session_config)
    .build();

let ctx: SessionContext = SessionContext::standalone_with_state(state).await?;

In this case logical and physical codec will be also be propagated to standalone.

Lastly, BallistaQueryPlanner can be replaced:

let session_config = SessionConfig::new_with_ballista()
    .with_information_schema(true)
    .set_str(BALLISTA_PLANNER_OVERRIDE, "false");

let state = SessionStateBuilder::new()
    .with_default_features()
    .with_config(session_config)
    .with_query_planner(Arc::new(BadPlanner::default()))
    .build();

let ctx: SessionContext = SessionContext::standalone_with_state(state).await?;

At the moment there is a hacky way telling ballista not to override provided planner with .set_str(BALLISTA_PLANNER_OVERRIDE, "false"); as it is not possible to detect if the planner is changed.

Are there any user-facing changes?

Introduction of two new methods and one extension to new functionality, no braking change

Notes:

  • Object store is not be set automatically anymore, will look into ballista_core::object_store_registry::with_object_store_registry deprecation in follow up commits when we expose configuration on scheduler and executor API
  • standalone_with_state may change slightly adding executor related configuration

@milenkovicm milenkovicm force-pushed the reloaded_session_context_config branch from fdb1419 to c49df1a Compare October 22, 2024 20:33
@milenkovicm milenkovicm changed the title Initial session store implementation for extensions Ergonomic way to setup/configure SessionContextExt Oct 22, 2024
Copy link
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice. Thanks @milenkovicm

@milenkovicm
Copy link
Contributor Author

milenkovicm commented Oct 25, 2024

could this change get merged please @andygrove , i have follow up #1099 on top of this one

@andygrove andygrove merged commit 5226595 into apache:main Oct 26, 2024
19 checks passed
@milenkovicm milenkovicm deleted the reloaded_session_context_config branch October 26, 2024 14:09
@milenkovicm
Copy link
Contributor Author

Thanks @andygrove

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ergonomic way to setup/configure SessionContextExt
2 participants