For that matter, it would be nice to reach a consensus on how to handle root credentials during bootstrapping, as it will be critical to running Polaris in isolated JVMs during integration tests.
So please review my latest changes in https://github.com/apache/polaris/pull/422 Thanks, Dmitri. On Wed, Nov 13, 2024 at 4:28 PM Dmitri Bourlatchkov < dmitri.bourlatch...@dremio.com> wrote: > To avoid confusion: I mean `polaris-test` is for the integration kind of > tests that require some support from the environment, like starting a > Polaris server. > > On Wed, Nov 13, 2024 at 4:27 PM Dmitri Bourlatchkov < > dmitri.bourlatch...@dremio.com> wrote: > >> Re: test classes - I'd propose to make a new module, e.g. `polaris-test` >> to contain abstract (actually or in spirit) test code, which would then be >> picked up by DW and Quarkus-specific actual test classes. >> >> I'm sure we can isolate env. details in JUnit5 extensions and inject >> relevant access objects (e.g. REST clients / URLs / etc) in runtime. >> >> Cheers, >> Dmitri. >> >> On Wed, Nov 13, 2024 at 2:13 PM Alex Dutra <alex.du...@dremio.com.invalid> >> wrote: >> >>> 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 >>> > > >> >>> > > > >>> > >>> >>
