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
