HonahX commented on code in PR #3200:
URL: https://github.com/apache/polaris/pull/3200#discussion_r2590994670
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/EntityResult.java:
##########
@@ -87,15 +85,6 @@ public PolarisEntitySubType getAlreadyExistsEntitySubType() {
}
}
- @JsonCreator
- private EntityResult(
Review Comment:
Hi @dimas-b Thanks for sharing the thoughts!
IMHO this is not a requirement imposed by any downstream project. The JSON
serde behavior has been in Polaris since the very beginning and has effectively
become an existing convention in the core module. We can still see many similar
annotations across the codebase today:
-
[PolarisEntityId](https://github.com/apache/polaris/blob/d82eee682df15708afbb87d8bf5a186b6402b0d8/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntityId.java#L35)
-
[PolarisEntitySubType](https://github.com/apache/polaris/blob/d82eee682df15708afbb87d8bf5a186b6402b0d8/polaris-core/src/main/java/org/apache/polaris/core/entity/PolarisEntitySubType.java#L95)
- ...
In that sense, removing these private @JsonCreator constructors is not just
a cleanup — it changes the long-standing implication that these classes are
expected to be JSON-serializable. Hence, I feel the situation is different from
the dev ML conversion linked above
Regarding potential use cases: the downstream integration example I
mentioned was just one illustration. Other scenarios, such as migrating or
syncing principals or entities between Polaris deployments, could also
reasonably rely on these classes being JSON serde.
In general, I would prefer that we not lose this established assumption and
flexibility purely for the sake of having less code. If the private
constructors themselves can be considered confusing, we could replace them with
static json creator, and switching to builder/Immutable for result classes for
example.
Please let me know WDYT!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
