I’m happy to consider the rest of the ecosystem plugins, though I would like to decouple that from the CDI stuff. As I’ve mentioned before, I’m all for improving the customizability of the codebase and I love the support for more flexible implementations of the various components. That’s something I think we can agree on and move forward with in a way that’s compatible with any future framework choice.
I still think the framework migration is a sizable task and I think the pros and cons of moving forward with such a migration should be written down and considered carefully. I did start that table a while back with some of the details. Maybe we can make some progress on filling it out with more details? I’m really interested in knowing specifics about which framework plugins will work, how much unnecessary code we can get rid of and what new capabilities we’d be able to take advantage of. I know there are a lot of vague promises that sound enticing, but I’ve also been bitten by those in the past, so I’m a bit more wary than I once was ;) Some of the prospects, especially OIDC support, are super interesting to me, but I just don’t know the full set of details well enough to weigh the pros and cons. Mike On Mon, Nov 25, 2024 at 10:41 PM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > Hi Michael > > Thanks for the update. It's great ! > Using CDI annotation is way neater than yaml bean verbose definition > :) We can always improve step by step (about the > PolarisMetaStoreManager and the CallContext). > > NB: the Quarkus PR is also there to have side/side comparison. > > My comment is that we should also consider the framework ecosystem: I > see new Polaris PRs (like the one about dynamic config) that > reimplement from scratch what already provided by Quarkus (thanks to > the extensions). I think it's urgent to have a consensus here to know > in which direction we are going. > > Let's chat together about that. > > Thanks ! > Regards > JB > > On Tue, Nov 26, 2024 at 6:06 AM Michael Collado <collado.m...@gmail.com> > wrote: > > > > FYI, I updated the branch to exclude HK2 dependencies from the core > module > > - > > > https://github.com/apache/polaris/compare/main...collado-mike:polaris:mcollado-hk2-di > > . The Factory implementations are defined in the polaris-service module. > > > > I also created another branch at > > > https://github.com/collado-mike/polaris/compare/mcollado-hk2-di...mcollado-hk2-di-grantmanager > > that merged the PR I have at https://github.com/apache/polaris/pull/465 > > using the CDI annotations rather than the extra factories I had created > in > > the initial PR. This is still moving us toward supporting different > > implementations of the various Polaris*Manager interfaces - still relying > > on the current concrete PolarisMetaStoreManager, but we can easily extend > > the HK2 service-locator file to support different manager implementations > > in future PRs. (I prefer smaller incremental moves before we go with > > fully-swappable implementations). > > > > Mike > > > > On Tue, Nov 19, 2024 at 8:37 PM Michael Collado <collado.m...@gmail.com> > > wrote: > > > > > 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 > > >> > >> > > >> > > > > >> > > > >> > > > >
