eric-maynard commented on code in PR #461:
URL: https://github.com/apache/polaris/pull/461#discussion_r1860373265
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java:
##########
@@ -95,10 +97,26 @@ public synchronized Map<String, PrincipalSecretsResult>
bootstrapRealms(List<Str
RealmContext realmContext = () -> realm;
if
(!metaStoreManagerMap.containsKey(realmContext.getRealmIdentifier())) {
initializeForRealm(realmContext);
+ // While bootstrapping we need to act as a fake privileged context
since the real
+ // CallContext hasn't even been resolved yet.
+ PolarisCallContext polarisContext =
+ new PolarisCallContext(
+
sessionSupplierMap.get(realmContext.getRealmIdentifier()).get(), diagServices);
PrincipalSecretsResult secretsResult =
bootstrapServiceAndCreatePolarisPrincipalForRealm(
- realmContext,
metaStoreManagerMap.get(realmContext.getRealmIdentifier()));
+ realmContext,
+ metaStoreManagerMap.get(realmContext.getRealmIdentifier()),
+ polarisContext);
results.put(realmContext.getRealmIdentifier(), secretsResult);
+ if (this.printCredentials(polarisContext)) {
+ String msg =
+ String.format(
+ "realm: %1s root principal credentials: %2s:%3s",
+ realmContext.getRealmIdentifier(),
+ secretsResult.getPrincipalSecrets().getPrincipalClientId(),
+ secretsResult.getPrincipalSecrets().getMainSecret());
+ System.out.println(msg);
+ }
Review Comment:
It seems like the `PrincipalSecretsGenerator` is doing exactly what the name
suggests: generating secrets.
Whatever is done with those secrets -- persisting them, using them, printing
them -- is outside the purview of the generator itself.
You are right that the `MetaStoreManager` doesn't need to know anything
about printing either (it doesn't in this PR) and clearly this should be
outside the purview of the metastore itself.
And so we landed on the factory. I would be happy to take this bootstrapping
logic and excise it to somewhere more idiomatic if that is a concern. But right
now the bootstrapping logic lives here (e.g. the `purge` check) and this seems
like the most appropriate place that doesn't change the responsibility of
either the metastore or generator classes.
--
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]
