flyrain commented on code in PR #741:
URL: https://github.com/apache/polaris/pull/741#discussion_r1917538355


##########
polaris-core/src/main/java/org/apache/polaris/core/context/RealmId.java:
##########
@@ -18,17 +18,26 @@
  */
 package org.apache.polaris.core.context;
 
+import com.fasterxml.jackson.annotation.JsonValue;
+import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
+import com.fasterxml.jackson.databind.annotation.JsonSerialize;
+import org.apache.polaris.immutables.PolarisImmutable;
+import org.immutables.value.Value;
+
 /**
- * Represents the elements of a REST request associated with routing to 
independent and isolated
- * "universes". This may include properties such as region, deployment 
environment (e.g. dev, qa,
- * prod), and/or account.
+ * Represents the ID of the realm used in a REST request associated with 
routing to independent and
+ * isolated "universes".
  */
-public interface RealmContext {
+@PolarisImmutable
+@JsonSerialize(as = ImmutableRealmId.class)
+@JsonDeserialize(as = ImmutableRealmId.class)
+public interface RealmId {

Review Comment:
   The `realmId` used to be a simple string, but it has now been replaced by a 
`RealmId` type, which contains a field called `id`—essentially an `id` of an 
`id`. While this approach works, it feels cumbersome and lacks extensibility. 
For instance, if we want to add new fields like a `description` for a realm, we 
would have to either include it under this interface or create a separate 
interface, such as `Realm` or `RealmContext`. 
   
   Adding new fields directly under `RealmId` feels odd since these fields 
typically represent a concept that is part of a `realm` rather than the 
`realmId`. On the other hand, introducing a separate interface adds complexity 
by requiring an additional structure to be defined.
   
   Additionally, I’m questioning whether the concepts of `realm` and `realmId` 
are truly distinct or interchangeable. If they are interchangeable, using the 
name `realm` instead of `realmId` might be a more concise and descriptive 
choice, simplifying the naming and making the code easier to understand.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to