adutra commented on code in PR #3669:
URL: https://github.com/apache/polaris/pull/3669#discussion_r2769052709
##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java:
##########
@@ -59,43 +52,25 @@ public abstract class CatalogHandler {
// Initialized in the authorize methods.
protected PolarisResolutionManifest resolutionManifest = null;
- protected final ResolutionManifestFactory resolutionManifestFactory;
Review Comment:
I had this approach initially but changed for the Parameter Object pattern,
for a few reasons:
1. `CatalogHandler` and subclasses are inherently mutable. Fields like
`resolutionManifest` are lazily evaluated, but are hard to migrate to a
`@Value.Lazy` equivalent because they may be evaluated more than once.
2. Getters in `CatalogHandler` would have to be public, because it's in a
different package from its subclasses, and thus the generated builders only
compile if the getters are public.
That said, I don't mind showing my initial version, maybe it's acceptable
indeed. The advantage being, that we could look into removing the mutable state
little by little until the class hierarchy becomes fully immutable.
I will add a commit on top of this branch to show the approach.
--
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]
