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
> > > > >> > >>
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > >
>

Reply via email to