deniskuzZ commented on PR #6454:
URL: https://github.com/apache/hive/pull/6454#issuecomment-4326953124
> Ahh sorry, second misread. I think `setConf` is still needed here because
HMSCatalogFactory's `configuration` is the fully-populated HiveConf — HMS
thrift auth, SSL/timeout, kerberos, the `SERVLET_ID_KEY`, etc. Without
`setConf`, HiveCatalog falls back to its default Configuration and loses all of
that. The new properties-only path makes sense for callers that initialize
purely from a property map (IcebergSummaryHandler), but on this path setConf is
providing more than the URI/warehouse/external-warehouse triple. Open to
dropping it if there's a flow I'm missing — does HiveCatalog reconstruct the
HMS-specific config from properties alone somewhere I haven't traced?
FYI, Polaris calls just initialize. We already authenticated here, so i
don't think we need to pass anything extra.
````
final String configExtWarehouse = MetastoreConf.getVar(configuration,
MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL);
if (configExtWarehouse != null) {
// HiveCatalog reads this property directly from Configuration, not
from properties map
configuration.set(MetastoreConf.ConfVars.WAREHOUSE_EXTERNAL.getHiveName(),
configExtWarehouse);
}
````
is same crazy useless logic
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]