Hi Michael, Good point about calls from application-scoped objects into request-scoped objects! I believe it is rather an omission in the current code rather than a deliberate design decision :) Do you happen to have some specific examples handy? I'd like to review what's involved, but I do not recall what those cases are :)
Re: creating objects vs. passing context parameters, it's not a requirement of CDI nor is it in any way forced by CDI. It is more of a general design proposal. I believe keeping contextual information in object properties (java fields) allows for simpler and easier to use definitions of their interfaces. The goal is to keep method parameters trimmed down to what is necessary for the operation at hand, but avoid contextual parameters. If that idea in general does not resonate with you, then using CDI or any other runtime framework is really an irrelevant point (for this thread). If that idea seems acceptable in principle, I'm sure we can figure out a way to refactor core classes to be usable as a library without relying on any particular framework. WDYT? Thanks, Dmitri. On Thu, Oct 9, 2025 at 2:34 PM Michael Collado <[email protected]> wrote: > > I imagine it eventually comes down to creating java objects. What CDI can > do automatically, can be done without CDI too. > > > Why would passing CallContext to constructor parameters be significantly > harder than passing it to method parameters? > > I mean, in an extreme scenario, you could describe nearly everything as > just creating new objects and never pass arguments at all ;) But to > maintain abstractions, the caller shouldn't be responsible for constructing > the object it wants to call methods on. We use polaris-core as a library in > addition to running the service (and as I recall, we all agreed very early > on that managing polaris-core as a usable library was a supported use case > - this is why we maintain Java 11 compatibility for the core, but not for > other modules) and that environment isn't CDI-capable. To now require that > every request construct a new instance of the PolarisMetaStoreManager on > every request because the CDI environment doesn't want to pass in one extra > argument puts extra burden on callers outside of the CDI environment. > Callers now have to be aware that the metastore manager is now > request-scoped - previously, it was possible to construct singleton > components (talking generally, not CDI-managed objects) with the > PolarisMetaStoreManager as a final field. Now, those classes need to > construct a new PolarisMetaStoreManager on each method invocation. Or their > API needs to change to accept a PolarisMetaStoreManager as an argument. > Such a change to the lifecycle is only easy in a CDI environment because > proxies allow for application-scoped components to use request-scoped > components without any code changes. In any other environment, it's a major > API change. > > Mike > > On Wed, Oct 8, 2025 at 4:39 PM Honah J. <[email protected]> wrote: > > > Thanks for the proposal and the effort of this refactoring, Christopher! > > Really appreciate the detailed summary of pros and cons in the document > > > > I share some of Michael’s concerns and wanted to add a bit more > > perspective: > > > > 1. Removal of CallContext from PolarisMetastoreManager > > As also mentioned in the proposal, removing CallContext completely means > > that custom implementations can no longer pass additional > request-specific > > information into the persistence layer.Since all requests eventually go > > through the persistence APIs, allowing request-specific or custom > metadata > > to be passed in could help keep the interface more extensible and > adaptable > > for future use cases. > > > > 2. Making PolarisMetaStoreManager request-scoped > > IMHO, one advantage of keeping the PolarisMetaStoreManager > > application-scoped is that it can hold state independent of individual > > requests. For example, if someone wants to back the metastore with a > > REST-based persistence service, the manager could maintain a shared HTTP > > client pool at the application level and serialize operations over REST. > > We’ve already seen similar REST-based integration patterns, such as in > > OpaPolarisAuthorizer. In general, I think interfaces like persistence, > > auth, and authorizer would benefit from remaining framework-agnostic and > > flexible for REST-style integrations — where parameters carry all > > request-specific information, while the instance itself holds > > request-independent state for optimization. > > > > Love to hear others' thoughts on this! > > > > Best Regards, > > Jonas > > > > On Wed, Oct 8, 2025 at 6:08 PM Dmitri Bourlatchkov <[email protected]> > > wrote: > > > > > Hi Michael, > > > > > > Could you share some more details about non-CDI environments? What > would > > be > > > the basic approach to creating services and related objects in runtime? > > > > > > I imagine it eventually comes down to creating java objects. What CDI > can > > > do automatically, can be done without CDI too. > > > > > > Why would passing CallContext to constructor parameters be > significantly > > > harder than passing it to method parameters? > > > > > > In my mind, using constructors (by extension class fields) for setting > > > contextual information is actually easier than passing it around in > every > > > method call. > > > > > > Regarding avoiding CDI in core classes, IIRC it was about CDI > annotations > > > (my recollection may be skewed, though). This PR does not introduce CDI > > > annotations into core. The core classes are still quite usable in > non-CDI > > > environments, so I guess this cycles back to the convenience point > > (above). > > > > > > Re: PolarisMetaStoreManager being stateless, I do not see any material > > > change in that regard. In fact, I think this PR makes it more stateless > > by > > > reducing the lifespan of PolarisMetaStoreManager instances. > > > > > > Thanks, > > > Dmitri. > > > > > > On Wed, Oct 8, 2025 at 5:54 PM Michael Collado <[email protected] > > > > > wrote: > > > > > > > I thought we generally agreed to avoid CDI in the core classes. This > > > > refactoring makes using the metastore API substantially harder to use > > in > > > > non-CDI environments. It was an intentional design choice to make the > > > > PolarisMetaStoreManager itself stateless and carry all of the > required > > > > state in the PolarisCallContext. > > > > > > > > Mike > > > > > > > > On Tue, Oct 7, 2025 at 6:31 AM Dmitri Bourlatchkov <[email protected] > > > > > > wrote: > > > > > > > > > Thanks for this refactoring proposal, Christopher! > > > > > > > > > > I think it makes the code align better with CDI concepts and should > > > make > > > > it > > > > > easier to evolve the codebase. I fully support moving in this > > > direction. > > > > > > > > > > As for downstream projects, as far as I can tell, it should still > be > > > > > possible to inject custom request-scoped data. If other community > > > members > > > > > have concerns with this, we can certainly discuss and I'm sure we > can > > > > find > > > > > solutions. > > > > > > > > > > Cheers, > > > > > Dmitri. > > > > > > > > > > On Mon, Oct 6, 2025 at 11:41 AM Christopher Lambert > > > <[email protected] > > > > > > > > > > wrote: > > > > > > > > > > > Hello everyone, > > > > > > > > > > > > I have put up a PR that reworks "PolarisMetaStoreManager" to be > > > > > > request-scoped: > > > > > > > > > > > > https://github.com/apache/polaris/pull/2555 > > > > > > > > > > > > From my perspective this cleans up / improves the design around > > > > > > persistence access in the code base without sacrificing any > > > > flexibility. > > > > > > > > > > > > I have created a document to explain the reasoning and for > > discussion > > > > > here: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://docs.google.com/document/d/1T6nxpEKpHWvcqu2qTEaS516lmtJNfeXiHm96vNNZCvo/edit?usp=sharing > > > > > > > > > > > > Looking forward to your feedback and review. > > > > > > > > > > > > Thank you, Christopher > > > > > > > > > > > > > > > > > > > > > > > > > > >
