dimas-b commented on code in PR #1783:
URL: https://github.com/apache/polaris/pull/1783#discussion_r2124636829
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/PolarisMetaStoreManager.java:
##########
@@ -307,7 +308,10 @@ EntityResult loadEntity(
*/
@Nonnull
EntitiesResult loadTasks(
- @Nonnull PolarisCallContext callCtx, String executorId, PageToken
pageToken);
+ @Nonnull PolarisCallContext callCtx,
Review Comment:
`PolarisCallContext` and `RealmContext` together in the same method call do
raise an alarm in my head ;)
This is what I call "API skew" - both parameters are associated with a
realm, but there's no guarantee the realm ID is the same. Moreover, there's no
clarity what implementations should use for getting the realm ID.
`CallContext` might be acceptable in the short term if the ripple effects of
using it are not too large.
In general, I'd like Polaris code to get all "context" stuff injected via
CDI, though.
--
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
