-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Add some more meaningful names to distinct Bootstrap instances #27203
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdate Bootstrap instantiations across multiple plugins/connectors to include descriptive name prefixes (access, catalog, auth, groups, resource-group) for clearer context and debugging. Class diagram for updated Bootstrap instantiationsclassDiagram
class FileBasedSystemAccessControl {
+create(config, context)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.access." + getName()
}
class ExampleConnectorFactory {
+create(catalogName, requiredConfig, context)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.catalog." + catalogName
}
class FileAuthenticatorFactory {
+create(config)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.auth." + getName()
}
class FileGroupProviderFactory {
+create(config)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.groups." + getName()
}
class LdapAuthenticatorFactory {
+create(config)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.auth." + getName()
}
class SalesforceAuthenticatorFactory {
+create(config)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.auth." + getName()
}
class FileResourceGroupConfigurationManagerFactory {
+create(config, context)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.resource-group." + getName()
}
class DbResourceGroupConfigurationManagerFactory {
+create(config, context)
// Bootstrap now instantiated with name prefix: "io.trino.bootstrap.resource-group." + getName()
}
class Bootstrap {
+Bootstrap(name, ...modules)
}
FileBasedSystemAccessControl --> Bootstrap
ExampleConnectorFactory --> Bootstrap
FileAuthenticatorFactory --> Bootstrap
FileGroupProviderFactory --> Bootstrap
LdapAuthenticatorFactory --> Bootstrap
SalesforceAuthenticatorFactory --> Bootstrap
FileResourceGroupConfigurationManagerFactory --> Bootstrap
DbResourceGroupConfigurationManagerFactory --> Bootstrap
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
| // A plugin is not required to use Guice; it is just very convenient | ||
| Bootstrap app = new Bootstrap( | ||
| "io.trino.bootstrap.catalog." + catalogName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this in "only" an example, but it should show this as a best practice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I don't think that's me... |
wat |
Description
Follow-up to d01c5ac.
Additional context and related issues
This "invents" some more bootstrap name prefixes, specifically
io.trino.bootstrap.auth.andio.trino.bootstrap.resource-group..Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:
Summary by Sourcery
Assign context-specific name prefixes to Bootstrap instances across various plugin factories to improve identification
Enhancements: