I'd prefer to keep RealmId for just the ID then, and add a separate
Realm type when there's a use case for it in Polaris.
Custom realm-specific additions can always produced when needed - e.g.
using `@Produces @RequestScoped MyRealmData produceMyRealmData(RealmId
id, OtherBeanOne one, AnotherBean two)` - and then injected where it's
needed. That allows an extension to reuse existing realm-resolution
mechanisms.
As mentioned in my previous post, I'm concerned about custom code that
extends a Polaris interface. That approach requires to hook into all
existing code that produces a (not yet existing) `Realm` type - it would
have to replace it everywhere - even in components that an extension
does not know about. In other words: that approach makes an extension
mechanism of Polaris really difficult.
On 27.01.25 17:58, Dmitri Bourlatchkov wrote:
Good point about entity injection. I still have reservations, but I guess
we can address any specific issues when we hit them.
If we go with entity injection, I'd think the refactoring has to go beyond
the class/interface renaming and aim at removing `Realm` method parameters.
I believe Realm will have to be an instance field on the affected beans.
Consequently, all beans referencing Realm will have to become
request-scoped (if they are not such already).
Cheers,
Dmitri.
On Mon, Jan 27, 2025 at 11:10 AM Alex Dutra <alex.du...@dremio.com.invalid>
wrote:
Hi all,
I guess we need to align our vision of what "RealmId" (or whatever we call
it eventually) actually represents.
Some of us seem to consider that this component is really just the unique
identifier of a realm entity, pretty much like a primary key. Under that
vision, the component is not expected to exhibit any other state and is and
will ever be a wrapper around the realm's string id.
Others tend to consider that this component represents the whole realm
entity, thus potentially exposing more properties in the future (I
mentioned the "state" property as an example of that).
I personally would prefer this component to be the full entity, injected as
such. This is to avoid injecting things such as primary keys / identifiers,
forcing consumers to lookup the entity at some point to know more about it.
If we could just inject a proxy to the whole entity directly, that's imho
easier.
Thanks,
Alex
On Mon, Jan 27, 2025 at 4:05 PM Dmitri Bourlatchkov <di...@apache.org>
wrote:
Thanks for starting this discussion, Yufei! I think it is important to
clarify this sooner rather than later.
From my POV option 2 (String) is cumbersome. The biggest reasons being
more
complex CDI injection (as already noted by other people) but also more
complicated usage searches in IDEs.
As for option 1, I support the renaming to "realm" in CLI options and
other
user-settable parameters (such as config options). My thinking is that
from
the user's perspective, the intent is clear that the user wants to
select a
particular realm by its ID (or name).
As for java class changes, RealmId makes more sense to me because it
indicates that the object holds the ID and not the whole realm (as I
commented on the PR).
If, for example, we're going to manage Realms as first-class entities in
Polaris, there will be a need to represent administrative realm data in
memory. Then, there will have to make a distinction between Real ID
extracted from request parameters and Realm object holding persisted
data.
That said, I do not feel too strongly about renaming RealmId to Realm if
other people agree on that.
Cheers,
Dmitri.
On Fri, Jan 24, 2025 at 8:51 PM Yufei Gu <flyrain...@gmail.com> wrote:
Hi Folks
I wanted to share my thoughts on the ongoing discussion in PR #741
<https://github.com/apache/polaris/pull/741>. about whether to use
Realm
or
RealmId.
The more I consider it, the more it seems that the name Realm is the
more
natural choice. It is more an atomic concept, much like the concept of
a
region in AWS.
If we take a closer look at the current usage across Polaris—in
documentation, error messages, and configurations—realm and realmId are
already used interchangeably. In fact, in most cases, we simply use
realm
rather than realmId.
Here are a few examples in the Polaris repo:
Error Messages:
- realm: <realm> root principal credentials:
<client-id>:<client-secret>
Configurations:
- Polaris.realm-context.realms
- jdbc:h2:file:./build/test_data/polaris/{realm}/db
Documentation:
<https://polaris.apache.org/in-dev/unreleased/metastores/>
<https://polaris.apache.org/in-dev/unreleased/metastores/>
- <https://polaris.apache.org/in-dev/unreleased/metastores/
Metastores
<https://polaris.apache.org/in-dev/unreleased/metastores/>
- Configuring Polaris for Production
<
https://polaris.apache.org/in-dev/unreleased/configuring-polaris-for-production/
- Admin Tool <
https://polaris.apache.org/in-dev/unreleased/admin-tool/>
Proposal
Based on this consistency and the conceptual clarity it brings, I
proposed
two options:
Option 1, using the name realm instead of realmId throughout Polaris
and
still keeping the interface (public interface Realm) for dependency
injection purposes. The interface can be extensible in case we want to
add
any subconcept to the realm, which may never happen to be honest.
Option 2, purely using a string for realm instead of an interface, this
is
simpler ultimately, but not extensible and needs more effort to
refactor
the current code.
Looking forward to hearing your thoughts on this.
Yufei
--
Robert Stupp
@snazy
