gagan405 commented on issue #7251: URL: https://github.com/apache/gravitino/issues/7251#issuecomment-2917362719
Thanks for the context. I think in principle, it is okay to keep the cache static as it is expected to be initialized once in its lifetime. But what goes against the norm here is having the cache static, and initializing it through the constructor. It works fine as it is instantiated once as part of the server startup. But it does get some code smell in unit tests for example, where the CatalogManager has to be constructed first, and then the static variable (cache) is accessed. Nothing prevents from doing `CatalogManager.cache` but that will fail, if `new CatalogManager()` wasn't done first. This kind of breaks the contract. I can create a small refactor to this and fix it if that makes sense. -- 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]
