Having a consistent name across the system is a huge benefit for users and
developers. There would be less confusion and less cognitive burden for
everyone. Otherwise, people may keep thinking "realm" and "realmId" are
different things. As I said in the proposal, the name "realm" is already
widely used. I don't think we are going to change them to `realmId`.
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/>
Yufei
On Wed, Jan 29, 2025 at 1:17 PM Robert Stupp <sn...@snazy.de> wrote:
> Thinking more about this, having a huge change just for a nomenclature
> thing is really not worth the effort, so I'm against such a rename.
>
> On 29.01.25 11:02, Robert Stupp wrote:
> > Let's take a step back.
> >
> > The documented contract of the old `RealmContext` was to provide the
> > realm-ID from a REST request - nothing more, nothing less - that's all
> > what's been implemented. And this is exactly the reason why the
> > current type is called `RealmId`.
> >
> > Renaming `RealmId` to `Realm` and then also add another `RealmXyz`
> > type makes it difficult to distinguish, causing unnecessary more
> > confusion.
> >
> > Please keep in mind that the code that extracts the realm-ID from a
> > REST request needs to be self-contained. Adding more functionality to
> > that unnecessarily complicates the code to extract the realm-ID from a
> > REST request.
> >
> > If more information is needed, there can always be a CDI-producer that
> > provides that in another request-scoped bean based on `RealmId`.
> >
> > On 29.01.25 00:59, Yufei Gu wrote:
> >> Hi Folks,
> >>
> >> Thanks for chiming in! I agree that renaming while keeping the interface
> >> intact is the better option. I see realm more as an atomic concept, and
> >> renaming helps maintain consistency across the system. Please take a
> >> look
> >> at this renaming PR: https://github.com/apache/polaris/pull/899.
> >>
> >> Since the topic of extending the Realm interface came up, here are my
> >> thoughts:
> >> There are two potential approaches for extension:
> >> 1. Adding new fields directly to the Realm interface.
> >>
> >> 1. Works well for private or generated fields.
> >> 2. Public fields can also be added carefully, though I share
> >> Robert’s
> >> concern that some custom beans may not include the new fields. In
> >> that
> >> case, we should make sure that existing functionality remains
> >> intact, even
> >> if new fields are empty, so that the backward compatibility is
> >> still kept.
> >>
> >> 2. Creating a new entity that wraps around Realm. This avoids modifying
> >> Realm directly but requires updating all interfaces that interact
> >> with the
> >> new entity, making it also a significant change.
> >>
> >> Yufei
> >>
> >> On Mon, Jan 27, 2025 at 9:37 AM Robert Stupp <sn...@snazy.de> wrote:
> >>
> >>> 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
> >>>
> >>>
> --
> Robert Stupp
> @snazy
>
>
