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