Hi Michael, hi all, I had a look at your branch and am very pleased to see that it was relatively easy to get DI working with DW + HK2. Good news!
Digging a bit into the details, I see that most beans are annotated with @Named now. This is probably OK for Quarkus too, although there are other techniques that allow to select a specific implementation at runtime. I also think that the @RealmContext annotation is very smart, and resonates with how I handled that in Quarkus. Although, I wonder if this custom context isn't identical to the standard request context? IOW, is it possible for the Polaris context to change during an HTTP call to Polaris API? I don't think so, which leads me to think that, at least in Quarkus, we'd use the standard @RequestScoped annotation for passing things like this. But now taking a step back and looking at the big picture, I think that what we need now is a plan forward. Given that many of us voiced concerns around an all-or-nothing switch from DW to Quarkus (quite understandably so), and assuming that there is agreement to maintain both runtimes (DW and Quarkus) for some time, I'd like to suggest the phased approach below: 1. Remove the dependency to DW from polaris-core – there is a PR for that already: https://github.com/apache/polaris/pull/435 2. Refactor polaris-service into 2 modules: e.g. "polaris-service-common" and "polaris-service-dropwizard" – I believe Dmitri is already working on this 3. Introduce DI + HK2 in polaris-service-dropwizard – that would come from your branch basically 4. Introduce polaris-service-quarkus – that would come mostly from JB's branch Regarding step #2, it seems we can easily put 80 to 90% of the actual polaris-service production classes into the common module. The dropwizard runtime module would contain just a handful of DW-specific classes. The biggest issue to solve with step #2 is imho test classes. Quarkus and DW have diverse requirements for tests, but we may still be able to refactor things using abstract classes or extensions, and reuse most of them. In any case, I think that if, at least at the beginning, test classes are duplicated in each runtime module, that's acceptable until we figure out a better way. And to circle back to DI, whether this common module would contain classes annotated with Jakarta CDI annotations is imho up for debate. This would simplify things for sure, but on the flip side, would require both runtimes to use the exact same CDI mechanisms (e.g. both runtimes would use @Named beans). I think that this is going to be too restrictive. An alternative would be to keep the common classes clean of CDI annotations and let the runtime modules create subclasses annotated with their own annotations. And back to the @RealmContext topic, that's the other big aspect that we will need to solve (maybe step #5?): CallContext is heavily reliant on ThreadLocal, and that is not very CDI-friendly, as you've seen. For now, I did a patchy workaround for Quarkus. But we'd need to refactor that code cleanly, either by creating a custom context as you did, or by making CallContext a request-scoped bean that is evaluated for each HTTP request. Either way, it then needs to be propagated to other threads (e.g. in TaskExecutorImpl); for Quarkus, this will require using proper context propagation <https://quarkus.io/guides/context-propagation> (for DW, maybe ThreadLocal is fine here). And since CallContext is in the core module, we will likely need to remove the static methods getCurrentContext() and setCurrentContext() and retrieve the context only by scope resolution (which, in turn, may use ThreadLocal under the hood if that's appropriate for DW). If everybody is on board with the plan above, how about we create GH issues for each step, and maybe even a GH Project to track progress on this front and split the effort? Thanks, Alex On Sat, Nov 9, 2024 at 10:15 AM Jean-Baptiste Onofré <j...@nanthrax.net> wrote: > Hi Michael > > Thanks for the update ! > > As discussed during the last community meeting: > 1. I will open the draft PR with Quarkus in the meantime > 2. I will take a look on your branch powered by Glassfish HK2 CDI > (give me a few days as I was travelling this week :)) > > Thanks ! > Regards > > On Fri, Nov 8, 2024 at 2:36 AM 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 > > >> > > > >
