gianm commented on a change in pull request #9279: Guicify druid sql module
URL: https://github.com/apache/druid/pull/9279#discussion_r374407723
##########
File path: sql/src/main/java/org/apache/druid/sql/guice/SqlModule.java
##########
@@ -63,25 +55,21 @@ public void configure(Binder binder)
if (isEnabled()) {
Calcites.setSystemProperties();
- JsonConfigProvider.bind(binder, "druid.sql.planner",
PlannerConfig.class);
- JsonConfigProvider.bind(binder, "druid.sql.avatica",
AvaticaServerConfig.class);
- LifecycleModule.register(binder, DruidSchema.class);
binder.bind(ViewManager.class).to(NoopViewManager.class).in(LazySingleton.class);
- // Add empty SqlAggregator binder.
- Multibinder.newSetBinder(binder, SqlAggregator.class);
+ binder.install(new DruidCalciteSchemaModule());
Review comment:
The only reason I asked was because I was concerned that having things be
too split up would make it hard for people to guess what functionality should
be in which file (at least with something named SqlModule it's clear that all
the SQL stuff goes in there).
The easier testability is nice though, so you convinced me it's worth it.
Let's just make sure it's as clear as possible from the namings what stuff goes
in what module (or at least from the javadocs).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
