gh-yzou commented on code in PR #2294:
URL: https://github.com/apache/polaris/pull/2294#discussion_r2274938648
##########
runtime/service/src/main/java/org/apache/polaris/service/task/TaskExecutorImpl.java:
##########
@@ -122,27 +131,58 @@ public void addTaskHandlerContext(long taskEntityId,
CallContext callContext) {
// the task is still running.
// Note: PolarisCallContext has request-scoped beans as well, and must be
cloned.
// FIXME replace with context propagation?
- CallContext clone = callContext.copy();
- tryHandleTask(taskEntityId, clone, null, 1);
+ tryHandleTask(taskEntityId, new TaskContext(callContext), null, 1);
+ }
+
+ record TaskContext(String realmId, PolarisDiagnostics diagnostics) {
Review Comment:
t_pyspark/test_spark_sql_s3_with_privileges.py can only be run locally with
your avaiable S3 account, @jbonofre is working on getting an S3 account for cli
testing.
However, I am not sure what would be the extra benefit this approach would
give us. The copy() method is used to propagate the whole call Context and
RealmContext to the task execution, and the id look up seems also only used by
task execution and requires an extra re-construction step. Furthermore, this
approach doesn't seem scalable in the future if we are adding more information
to callContext or RealmContext (such as user specific information), especially
for RealmContext, it may require us to implement a RealmContextManager for
lookup.
If the concern is that this function is only used by task executor, i recall
@dimas-b was POC something that leverages CDI feature to propagate the
CallContext to background thread, which seems a cleaner way to do that.
@dimas-b maybe we can resume that work?
--
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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
