This sounds great. I think we can agree on some strategies so that work can keep moving forward even as we're settling on the framework question:
1. Let's continue to rely on the interfaces we already have defined - e.g., the PolarisConfigurationStore is going to continue to be the mechanism for querying service/entity configuration even if we have a framework-specific underlying method for loading the configuration. 2. The core code is going to need to be framework-agnostic, so we can continue to define core-types and core-behavior without assuming the source of construction of those types (e.g., I think we can move forward with federated-identities without implementing any OIDC-specific implementation code). 3. Let's start incorporating the CDI annotations in a way that makes migrating to another framework seamless. This makes the code more usable now and also helps makes any migration easier For the first two points, I think it'll be important for us to be clear on what things are Polaris-specific rather than generic and possibly supplied by framework plugins. We should be clear about the behavior of Polaris-specific entities/operations (I'll update my federated-identities doc to reflect this). But if a change or introduction of a core type is necessary only because of a lack of framework support, it's a good thing to call out in the PR and to add to the list of pros/cons we have for the Quarkus evaluation. Do these points sound reasonable? Mike On Tue, Nov 26, 2024 at 1:46 AM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > Hi Mike > > Fully agree, I just wanted to remind that CDI is a good think, but > considering ecosystem/plugins is also key (and indeed decoupled from > CDI). > > I think we already have a kind of consensus about CDI, and agree to > move forward here with improvement of the current codebase. > > The reason why I mention runtime framework ecosystem/plugins is > because we already have some PRs/discussions related to that (and > somehow overlapping with runtime framework support): dynamic config, > metadata storage, OIDC, ... > By leveraging runtime framework capabilities, we will focus our effort > on the Polaris new/core features. > > +1 about populating the table and migration path. > > I will rebase the Quarkus PR and share a document with the tables you > created (let's use the Polaris proposal process ;)). > > Thanks ! > Regards > JB > > On Tue, Nov 26, 2024 at 8:24 AM Michael Collado <collado.m...@gmail.com> > wrote: > > > > 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 > > > > >> > >> > > > > >> > > > > > > >> > > > > > >> > > > > > > > > >
