HonahX commented on code in PR #3200:
URL: https://github.com/apache/polaris/pull/3200#discussion_r2593248699
##########
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. The way I see it, even though the JSON-serde paths aren’t
exercised in Polaris today, the @JsonCreator constructors have been there for
long time acting as an implicit signal that these classes are intended to
round-trip through JSON. In other words, the annotations themselves set an
expectation for how future changes to these classes should behave, regardless
of whether the current runtime calls into that path.
Whether we actually want to keep JSON serde as part of the core module’s
design in the future could be a separate discussion. It seems we may have
different views on that, which is totally fine — I think it’s something worth
discussing with the community so we can align on the long-term direction
together. I was about to raise a PR to restore JSON serde for PolarisEntityCore
(which was removed as a side effect of #1596), but that should depend on the
direction we decide collectively.
--
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]
