dimas-b commented on code in PR #1816:
URL: https://github.com/apache/polaris/pull/1816#discussion_r2129012375
##########
polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java:
##########
@@ -94,4 +94,17 @@ public RealmContext getRealmContext() {
public PolarisCallContext getPolarisCallContext() {
return this;
}
+
+ @Override
+ public CallContext copy() {
+ // The realm context is a request scoped bean injected by CDI,
+ // which will be closed after the http request. This copy is currently
+ // only used by TaskExecutor right before the task is handled, since the
+ // task is executed outside the active request scope, we need to make a
+ // copy of the RealmContext to ensure the access during the task executor.
+ String realmId = this.realmContext.getRealmIdentifier();
+ RealmContext realmContext = () -> realmId;
+ return new PolarisCallContext(
+ realmContext, this.metaStore, this.diagServices,
this.configurationStore, this.clock);
Review Comment:
By the logic of the comment makes sense to me in general.
However, I do not think it matters in practice ATM, because `RealmContext`
implementations are not really closable. Those objects are quite usable (via
latent references) even after the associated request context is destroyed.
This PR LGTM as an incremental improvement. In the future, I'd like to
manage all contextual data via injection (automated in Quarkus CDI runtime or
manual in customized env.)
##########
polaris-core/src/main/java/org/apache/polaris/core/PolarisCallContext.java:
##########
@@ -94,4 +94,17 @@ public RealmContext getRealmContext() {
public PolarisCallContext getPolarisCallContext() {
return this;
}
+
+ @Override
+ public CallContext copy() {
+ // The realm context is a request scoped bean injected by CDI,
+ // which will be closed after the http request. This copy is currently
+ // only used by TaskExecutor right before the task is handled, since the
+ // task is executed outside the active request scope, we need to make a
+ // copy of the RealmContext to ensure the access during the task executor.
+ String realmId = this.realmContext.getRealmIdentifier();
+ RealmContext realmContext = () -> realmId;
+ return new PolarisCallContext(
+ realmContext, this.metaStore, this.diagServices,
this.configurationStore, this.clock);
Review Comment:
The logic of the comment makes sense to me in general.
However, I do not think it matters in practice ATM, because `RealmContext`
implementations are not really closable. Those objects are quite usable (via
latent references) even after the associated request context is destroyed.
This PR LGTM as an incremental improvement. In the future, I'd like to
manage all contextual data via injection (automated in Quarkus CDI runtime or
manual in customized env.)
--
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]
