You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/04/24 11:07:19 UTC

[GitHub] [arrow-datafusion] alamb opened a new pull request, #6106: chore: Update api docs for `SessionContext`, `TaskContext`, etc

alamb opened a new pull request, #6106:
URL: https://github.com/apache/arrow-datafusion/pull/6106

   # Which issue does this PR close?
   
   Related to #3058 
   
   # Rationale for this change
   
   I am trying to make the API docs published to crates.io more useful and stand on their own
   
   # What changes are included in this PR?
   
   1. Improve the docs for `SessionContext` and consolidate some other docs from the `execution` module that were hard to discover
   
   # Are these changes tested?
   Yes (with CI checks)
   
   # Are there any user-facing changes?
   
   Hopefully better docs


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


[GitHub] [arrow-datafusion] comphead commented on a diff in pull request #6106: chore: Update api docs for `SessionContext`, `TaskContext`, etc

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #6106:
URL: https://github.com/apache/arrow-datafusion/pull/6106#discussion_r1176955624


##########
datafusion/execution/src/runtime_env.rs:
##########
@@ -33,6 +33,15 @@ use url::Url;
 
 #[derive(Clone)]
 /// Execution runtime environment.
+///
+/// A [`RuntimeEnv`] can be created from a [`RuntimeConfig`] and
+/// stores state to be shared across multiple sessions. In most

Review Comment:
   Probably I'm wrong here, but not sure RuntimeEnv is shared between sessions.
   The runtime will be shared between queries within single session.
   But `SessionContext::new()` creates new session with new UUID and new runtime.



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


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6106: chore: Update api docs for `SessionContext`, `TaskContext`, etc

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6106:
URL: https://github.com/apache/arrow-datafusion/pull/6106#discussion_r1175130622


##########
datafusion/core/src/execution/mod.rs:
##########
@@ -15,30 +15,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-//! This module contains the shared state available at different parts

Review Comment:
   this was hard to find, so I moved the content on to the structs that were referenced



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


[GitHub] [arrow-datafusion] alamb merged pull request #6106: chore: Update api docs for `SessionContext`, `TaskContext`, etc

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #6106:
URL: https://github.com/apache/arrow-datafusion/pull/6106


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


[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #6106: chore: Update api docs for `SessionContext`, `TaskContext`, etc

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #6106:
URL: https://github.com/apache/arrow-datafusion/pull/6106#discussion_r1177729222


##########
datafusion/execution/src/runtime_env.rs:
##########
@@ -33,6 +33,15 @@ use url::Url;
 
 #[derive(Clone)]
 /// Execution runtime environment.
+///
+/// A [`RuntimeEnv`] can be created from a [`RuntimeConfig`] and
+/// stores state to be shared across multiple sessions. In most

Review Comment:
   This is a good point -- by default a `SessionContext` creates a new instance of `RuntimeEnv` -- however that means any limits are not enforced across queries. I think this comment was trying to say if you want limits enforced across multiple session contexts you need to use the same RuntimeEnv
   
   I have tried to clarify this in  0bbf68718e



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


[GitHub] [arrow-datafusion] comphead commented on a diff in pull request #6106: chore: Update api docs for `SessionContext`, `TaskContext`, etc

Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #6106:
URL: https://github.com/apache/arrow-datafusion/pull/6106#discussion_r1178012180


##########
datafusion/core/src/execution/context.rs:
##########
@@ -259,13 +259,29 @@ impl SessionContext {
         Ok(())
     }
 
-    /// Creates a new `SessionContext` using the provided session configuration.
+    /// Creates a new `SessionContext` using the provided
+    /// [`SessionConfig`] and a new [`RuntimeEnv`].
+    ///
+    /// See [`Self::with_config_rt`] for more details on resource
+    /// limits.
     pub fn with_config(config: SessionConfig) -> Self {
         let runtime = Arc::new(RuntimeEnv::default());
         Self::with_config_rt(config, runtime)
     }
 
-    /// Creates a new `SessionContext` using the provided [`SessionConfig`] and [`RuntimeEnv`].
+    /// Creates a new `SessionContext` using the provided
+    /// [`SessionConfig`] and a [`RuntimeEnv`].
+    ///
+    /// # Resource Limits
+    ///
+    /// By default, each new `SessionContext` creates a new
+    /// `RuntimeEnv`, and therefore will not enforce memory or disk
+    /// limits for queries run on different `SessionContext`s.
+    ///
+    /// To enforce resource limits (e.g. to limit the total amount of

Review Comment:
   great, thanks



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