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