alamb commented on code in PR #13635:
URL: https://github.com/apache/datafusion/pull/13635#discussion_r1868520414
##########
benchmarks/src/bin/external_aggr.rs:
##########
@@ -195,10 +196,15 @@ impl ExternalAggrConfig {
let query_name =
format!("Q{query_id}({})", human_readable_size(mem_limit as
usize));
let config = self.common.config();
- let runtime_config = RuntimeConfig::new()
+ let runtime_env = RuntimeEnvBuilder::new()
.with_memory_pool(Arc::new(FairSpillPool::new(mem_limit as usize)))
.build_arc()?;
- let ctx = SessionContext::new_with_config_rt(config, runtime_config);
+ let state = SessionStateBuilder::new()
Review Comment:
this follows the new builder style model more fully
##########
datafusion/core/src/datasource/file_format/csv.rs:
##########
@@ -984,12 +983,10 @@ mod tests {
async fn query_compress_data(
file_compression_type: FileCompressionType,
) -> Result<()> {
- let runtime = Arc::new(RuntimeEnvBuilder::new().build()?);
Review Comment:
This simply creates a default RuntimeEnv which the SessionStateBuilder
already does, so there is no need to do it again
##########
datafusion/execution/src/task.rs:
##########
@@ -54,9 +52,7 @@ pub struct TaskContext {
impl Default for TaskContext {
fn default() -> Self {
- let runtime = RuntimeEnvBuilder::new()
- .build_arc()
- .expect("default runtime created successfully");
+ let runtime = Arc::new(RuntimeEnv::default());
Review Comment:
this does the same thing, just makes the intent clearer
##########
datafusion/execution/src/runtime_env.rs:
##########
@@ -66,28 +85,16 @@ impl Debug for RuntimeEnv {
}
impl RuntimeEnv {
- #[deprecated(since = "43.0.0", note = "please use `try_new` instead")]
+ #[deprecated(since = "43.0.0", note = "please use `RuntimeEnvBuilder`
instead")]
+ #[allow(deprecated)]
pub fn new(config: RuntimeConfig) -> Result<Self> {
Self::try_new(config)
}
/// Create env based on configuration
+ #[deprecated(since = "44.0.0", note = "please use `RuntimeEnvBuilder`
instead")]
+ #[allow(deprecated)]
pub fn try_new(config: RuntimeConfig) -> Result<Self> {
- let RuntimeConfig {
Review Comment:
this code is literally the same as RuntimeConfig::build so just call that
directly
##########
datafusion/core/src/execution/context/mod.rs:
##########
@@ -1932,14 +1931,12 @@ mod tests {
let path = path.join("tests/tpch-csv");
let url = format!("file://{}", path.display());
- let runtime = RuntimeEnvBuilder::new().build_arc()?;
Review Comment:
likewise here, this is the default runtime env, it isn't needed
##########
datafusion-cli/src/main.rs:
##########
@@ -156,26 +156,22 @@ async fn main_inner() -> Result<()> {
session_config = session_config.with_batch_size(batch_size);
};
- let rt_config = RuntimeConfig::new();
- let rt_config =
- // set memory pool size
- if let Some(memory_limit) = args.memory_limit {
- // set memory pool type
- match args.mem_pool_type {
- PoolType::Fair => rt_config
-
.with_memory_pool(Arc::new(FairSpillPool::new(memory_limit))),
- PoolType::Greedy => rt_config
-
.with_memory_pool(Arc::new(GreedyMemoryPool::new(memory_limit)))
- }
- } else {
- rt_config
+ let mut rt_builder = RuntimeEnvBuilder::new();
Review Comment:
I think this code is now clearer -- previously the `rt_config` was actually
a builder and used as such, but that was someone confusing given the code style
##########
datafusion/execution/src/runtime_env.rs:
##########
@@ -41,13 +41,32 @@ use url::Url;
/// Execution runtime environment that manages system resources such
/// as memory, disk, cache and storage.
///
-/// A [`RuntimeEnv`] is created from a [`RuntimeEnvBuilder`] and has the
+/// A [`RuntimeEnv`] can be created using [`RuntimeEnvBuilder`] and has the
/// following resource management functionality:
///
/// * [`MemoryPool`]: Manage memory
/// * [`DiskManager`]: Manage temporary files on local disk
/// * [`CacheManager`]: Manage temporary cache data during the session lifetime
/// * [`ObjectStoreRegistry`]: Manage mapping URLs to object store instances
+///
+/// # Example: Create default `RuntimeEnv`
Review Comment:
new doc examples
--
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]
