Hi again,

I understand that we haven't reached a full consensus on this topic
yet. But since some people showed interest in seeing it implemented, I
will go ahead and prepare the PR so that we can see what it would look
like.

More specifically I will:

- Merge the 2 modules
- Rename all the packages in the resulting module having a ".quarkus." token
- Rename all the classes in the resulting module having a "Quarkus" prefix

Thanks,
Alex

On Thu, Jul 31, 2025 at 8:11 PM Alexandre Dutra <adu...@apache.org> wrote:
>
> Hi Dmitri,
>
> Thanks for your reply. My proposal doesn’t affect how 
> polaris-runtime-defaults is “pulled in” by downstream builds.
>
> We can, however, explore ways to make the downstream integration experience 
> better, but imo only *after* the merge.
>
> Thanks,
> Alex
>
> Le jeu. 31 juil. 2025 à 19:56, Dmitri Bourlatchkov <di...@apache.org> a écrit 
> :
>>
>> Hi Alex,
>>
>> Unifying polaris-service-common and polaris-runtime-service sounds good to
>> me.
>>
>> Re: config, I suppose it should not be an issue to have Quarkus (or
>> Smallrye) dependencies in any "service" modules.
>>
>> Side note: I'd like to exclude polaris-runtime-defaults from the transitive
>> dependency chain and only depend on it directly from leaf runtime artifacts
>> (admin and server). The reason for this is to simplify downstream builds
>> that reuse common services (having multiple application.properties in a
>> Quarkus build env. is a nightmare :) ). I hope this will not interfere with
>> your proposal. I'm mentioning it here only because it affects
>> the polaris-runtime-service module (IIRC).
>>
>> Cheers,
>> Dmitri.
>>
>> On Thu, Jul 31, 2025 at 1:36 PM Alexandre Dutra <adu...@apache.org> wrote:
>>
>> > > I mentioned smallrye-config, not Quarkus, so you _can_ annotate those.
>> >
>> > Fair point: you are absolutely right. We *could* do things the other
>> > way around, and move the configuration classes from the
>> > polaris-runtime-service module to the polaris-service-common module.
>> >
>> > BUT: my main point for proposing this change still holds: *the two
>> > modules do not have a clearly defined purpose*. Both have code for
>> > e.g. authentication, events, persistence, storage, task handling, etc.
>> > etc.
>> >
>> > This doesn't look to me as a desirable state. I think that if classes
>> > that perform the same actions could live next to each other, that
>> > would be beneficial to the developer experience.
>> >
>> > Thanks,
>> > Alex
>> >
>> > On Thu, Jul 31, 2025 at 6:46 PM Robert Stupp <sn...@snazy.de> wrote:
>> > >
>> > > > That's not possible because there are no Quarkus dependencies in that
>> > module, so you can't annotate with @WithDefault, for example.
>> > >
>> > > I mentioned smallrye-config, not Quarkus, so you _can_ annotate those.
>> > >
>> > > On Thu, Jul 31, 2025 at 6:34 PM Yufei Gu <flyrain...@gmail.com> wrote:
>> > > >
>> > > > +1. Thanks Alex for driving this. Other than the benefits discussed
>> > like
>> > > > removing the duplicated config classes like
>> > > > `QuarkusStorageCredentialCacheConfig`, or removing `.quarkus.` in the
>> > > > package name, we can finally put classes like S3AccessConfig
>> > > > and StsClientsPool into the right package.
>> > > >
>> > > > Yufei
>> > > >
>> > > >
>> > > > On Thu, Jul 31, 2025 at 8:45 AM Eric Maynard <eric.w.mayn...@gmail.com
>> > >
>> > > > wrote:
>> > > >
>> > > > > The project already *has* adopted Quarkus for good, so I think
>> > making the
>> > > > > module structure reflect that is fine. In addition to the
>> > configuration
>> > > > > issue, which is significant, I think cutting down on the number of
>> > modules
>> > > > > we publish is a noble goal.
>> > > > >
>> > > > > —EM
>> > > > >
>> > > > > On Fri, Aug 1, 2025 at 00:43 Alexandre Dutra <adu...@apache.org>
>> > wrote:
>> > > > >
>> > > > > > Hi Robert,
>> > > > > >
>> > > > > > > I think we can get away by having the smallrye-config
>> > annotations on
>> > > > > the
>> > > > > > "parent" interface.
>> > > > > >
>> > > > > > That's not possible because there are no Quarkus dependencies in
>> > that
>> > > > > > module, so you can't annotate with @WithDefault, for example.
>> > > > > >
>> > > > > > Indeed merging those two modules would mean that we're adopting
>> > > > > > Quarkus for good, but I think that at this point, nobody would
>> > object.
>> > > > > >
>> > > > > > In this merge, we could also remove the ".quarkus." bits from
>> > package
>> > > > > > names and remove the "Quarkus" prefix of many classes. I think this
>> > > > > > would result in a much more readable code, but that's just my
>> > opinion
>> > > > > > :-)
>> > > > > >
>> > > > > > My long-term vision is that, after the merge, we could also
>> > consider
>> > > > > > splitting the resulting "uber-module" into smaller,
>> > concern-specific
>> > > > > > modules like "polaris-service-auth", "polaris-service-telemetry",
>> > > > > > "polaris-service-events", etc. This approach would make it much
>> > > > > > simpler for integrators to implement their own customizations (and
>> > > > > > brings no downsides for regular users).
>> > > > > >
>> > > > > > But this needs to be done in two steps: first, merge the current
>> > two
>> > > > > > modules; then, split the result.
>> > > > > >
>> > > > > > Thanks,
>> > > > > > Alex
>> > > > > >
>> > > > > > On Thu, Jul 31, 2025 at 5:22 PM Robert Stupp <sn...@snazy.de>
>> > wrote:
>> > > > > > >
>> > > > > > > Is the issue here just the configuration types or is there more?
>> > > > > > > For the config types, I think we can get away by having the
>> > > > > > > smallrye-config annotations on the "parent" interface.
>> > > > > > >
>> > > > > > > My concern is that polaris-runtime-service is rather the Quarkus
>> > > > > > specifics.
>> > > > > > > CDI is great, just Quarkus isn't the only enterprise-CDI runtime.
>> > > > > > > Spoiler: I do *not* think that Polaris should move away from
>> > Quarkus.
>> > > > > > >
>> > > > > > > But for sole testing purposes Quarkus is quite expensive, too
>> > > > > > > expensive IMO to make it a necessity for all tests.
>> > > > > > > Sure, not all tests have to be `@QuarkusTest`s, but I could
>> > imagine
>> > > > > > > that there will be tests that do not need Quarkus are annotated
>> > as
>> > > > > > > such.
>> > > > > > >
>> > > > > > > On Thu, Jul 31, 2025 at 12:19 PM Alexandre Dutra <
>> > adu...@apache.org>
>> > > > > > wrote:
>> > > > > > > >
>> > > > > > > > Hi all,
>> > > > > > > >
>> > > > > > > > The polaris-service-common module is a reminiscence of the
>> > times
>> > > > > where
>> > > > > > > > we were still supporting two runtimes.
>> > > > > > > >
>> > > > > > > > I think it has now become obsolete, and could be merged into
>> > > > > > > > polaris-runtime-service.
>> > > > > > > >
>> > > > > > > > One annoyance that polaris-service-common brings is with
>> > > > > configuration
>> > > > > > > > classes that have to exist in both modules (e.g.
>> > > > > > > > AuthenticationConfiguration vs
>> > QuarkusAuthenticationConfiguration).
>> > > > > > > >
>> > > > > > > > Would we be OK with this merge? If so I'm happy to provide the
>> > PR.
>> > > > > > > >
>> > > > > > > > Thanks,
>> > > > > > > > Alex
>> > > > > >
>> > > > >
>> >

Reply via email to