Hey thanks for calling that out. It’s strictly a convenience change and actually isn’t necessary. I added that in because, at the end of the day, most components need a PolarisMetaStoreManager, not a MetaStoreManagerFactory. Registering it as a factory meant that components could declare a dependency on the PolarisMetaStoreManager interface and it would use the factory to create the right instance. Because the factory method is annotated @RealmScope, the instance it creates is specific to the realm. I could have done it in the PolarisApplication, as I did the PolarisGrantManagerFactory at https://github.com/collado-mike/polaris/blob/mcollado-hk2-di/polaris-service/src/main/java/org/apache/polaris/service/PolarisApplication.java#L518-L531 . I was just lazy when I did that one :) We can keep all the hk2-specific code out of the core module.
I do want to point out where the @RealmScope differs from @RequestScope - the beans defined in @RequestScope are cleaned up at the end of the request. The ones defined in the @RealmScope are reused across requests. This makes a difference with classes like the EntityCache that depends on being able to reuse, e.g., catalogs, but ensure the cache is specific to the current realm. We can, of course, pass around a bunch of @ApplicationScope factories that return the realm-specific beans, but that means we can't do things like add the PolarisGrantManager interface without also adding a new realm-specific factory, even if the PolarisGrantManager implementation is really just the PolarisMetaStoreManager itself. I think, ideally, we avoid declaring dependencies on factories and have components declare dependencies on the specific realm-scoped beans they need and let the CDI framework work out where the beans come from. This is basically what I was doing with the MetaStoreManagerFactory interface - as I said, I was just being lazy by extending Factory directly. Mike On Tue, Nov 19, 2024 at 5:08 PM Dmitri Bourlatchkov <dmitri.bourlatch...@dremio.com.invalid> wrote: > I had a quite brief look at Mike's HK2 branch, so apologies if I'm missing > the bigger picture. > > Still, I see that MetaStoreManagerFactory no longer extends the DW > Discoverable, but it extends the HK2 Factory class now. So we're basically > trading one framework for another as a code-level runtime dependency. I > would really like it if we could avoid that. > > I think one of the key benefits of Quarkus is abstracting from framework > code in our class hierarchies. Granted, Quarkus requires certain Web App > frameworks, but that's at another level. As for CDI, our code under Quarkus > would only have to have certain annotations, without having to > extend/implement framework interfaces. > > Cheers, > Dmitri. > > On Thu, Nov 7, 2024 at 8:37 PM Michael Collado <collado.m...@gmail.com> > wrote: > > > FYI, I took a stab at seeing how Polaris would work with HK2 as a CDI > impl. > > I only spent yesterday and today on this, so it's not complete, but it is > > functional and the tests pass :) > > > > I took a lot of the common ideas from the Quarkus branch (e.g., deleted > all > > the HasXXX and XXXAware interfaces), but kept the JSON/Yaml config. I > > figured out how to use the Dropwizard Yaml to specify the > > implementation of, e.g. the Authenticator and the > MetaStoreManagerFactory, > > but have the instances managed and injectable by HK2. The goal there was > to > > just keep the existing configuration format, but change the impl under > the > > hood. I'm not married to the idea and I'm interested to see if the > > jakarta.enterprise.inject.* annotations/interfaces that are used in the > > Quarkus branch can make this simpler. However, I do think it would ideal > if > > we can get it working with the existing Yaml configuration, at least in > the > > short term. > > > > I did add support for a @RealmScope annotation to support restricting > items > > to a given realm, such as the EntityCache and the PolarisGrantManager. > This > > allowed me to do things like hide the grant lookups from the EntityCache > so > > that the Resolver doesn't have to pass around the ResolvedPolarisEntity, > > but instead the grants are found from the cache without making it overt > in > > the PolarisAuthorizer API. This was one of my original goals with > breaking > > up the PolarisMetaStoreManager API into multiple interfaces. Right now, > > everything still ties back to the configured PolarisMetaStoreManager > > implementation, but eventually we can get to where the GrantManager, > > CredentialVendor, etc. can all be swapped out for different > > implementations. > > > > Please take a look at the changes at > > > > > https://github.com/collado-mike/polaris/compare/c0e8dae5182d33e046216510e2b02b7cf923ffe8...collado-mike:polaris:mcollado-hk2-di > > and let me know what you think. > > > > Mike > > > > On Tue, Nov 5, 2024 at 9:42 AM Michael Collado <collado.m...@gmail.com> > > wrote: > > > > > I added a table to the README with the differences that were called > out. > > I > > > added some details that I think are worth understanding better. E.g., > the > > > Json layout we added has specific custom functionality we wanted for > > > supporting key/value pairs and the micrometer annotation was added for > > some > > > custom support we wanted aside from what is supported with the default > > > dropwizard metrics support. > > > > > > > > > > > > https://github.com/collado-mike/polaris/blob/14865a97ad8f790a9992432d79975c05ff5c36fa/polaris-service-quarkus/README.md > > > > > > I think it would be great if others could add to the table with > features > > > that would be impacted by the migration and to call out the level of > > effort > > > in both dropwizard and quarkus. > > > > > > Another possible consideration would be upgrading our Dropwizard > > > dependency to the latest development version. It may be the case that > > doing > > > so would address some of the targeted features with less effort in > > > migrating. > > > > > > Mike > > > > > > On Tue, Oct 29, 2024 at 5:50 AM Jean-Baptiste Onofré <j...@nanthrax.net> > > > wrote: > > > > > >> Hi folks, > > >> > > >> Following the last community sync meeting, we create a branch > > >> demonstrating use of Quarkus to powered Apache Polaris: > > >> > > >> https://github.com/jbonofre/polaris/tree/QUARKUS > > >> > > >> We still have work to do, but you can already take a look and > > >> experiment (in the polaris-service-quarkus module). > > >> > > >> We added a README.md file to: > > >> 1. Highlight the main differences (in terms of code) between > > >> Dropwizard and Quarkus > > >> 2. To build and run Polaris powered by Quarkus > > >> 3. The list of TODO items > > >> > > >> > > >> > > > https://github.com/jbonofre/polaris/blob/QUARKUS/polaris-service-quarkus/README.md > > >> > > >> If anyone wants to contribute on the branch before creating the PR, > > >> please let me know, I will add you as contributor on the branch. > > >> > > >> Any comments or questions are welcome ! > > >> > > >> Thanks, > > >> Regards > > >> JB > > >> > > > > > >
