Refactor origin inventory builder #78
Conversation
| public Builder connectionFactory(Connection.Factory connectionFactory) { | ||
| this.connectionFactory = connectionFactory; | ||
| public Builder originHealthMonitor(OriginHealthStatusMonitor originHealthMonitor) { | ||
| this.originHealthMonitor = originHealthMonitor; |
There was a problem hiding this comment.
We should check this for null here, to avoid throwing an exception in the build method.
| OriginsInventory originsInventory = new OriginsInventory(eventBus, backendService.id(), originHealthStatusMonitor, hostConnectionPoolFactory, metricsRegistry); | ||
| originsInventory.setOrigins(backendService.origins()); | ||
| OriginsInventory originsInventory = new OriginsInventory(eventBus, appId, originHealthMonitor, connectionPoolFactory, metricsRegistry); | ||
| originsInventory.setOrigins(initialOrigins); |
There was a problem hiding this comment.
I think the setOrigins(Set<Origin>) method can be private, and setOrigins(Origin...) can be package-private.
There was a problem hiding this comment.
Hi Kyle. The setOrigins(Set<Origin>) is nowadays public. This is because the inventory by design will manage the state for the origins, and the setOrigins provides a way to push more origins into the inventory.
| return new StatsReportingConnectionPool( | ||
| new SimpleConnectionPool( | ||
| origin, | ||
| defaultSettableConnectionPoolSettings(), |
There was a problem hiding this comment.
Should we rename defaultSettableConnectionPoolSettings since "settable" is no longer in the class name?
There was a problem hiding this comment.
Makes sense. Will do.
| @@ -124,6 +155,39 @@ public void onChange(Registry.Changes<BackendService> changes) { | |||
| .close()); | |||
| } | |||
|
|
|||
| private OriginHealthCheckFunction originHealthCheckFunction( | |||
There was a problem hiding this comment.
This method can be static.
| .build(); | ||
| return new ProxyToBackend(clientFactory.createClient(backendService, inventory)); | ||
| } | ||
|
|
||
| private ConnectionPoolFactory connectionPoolFactory( |
There was a problem hiding this comment.
This method can be static.
There was a problem hiding this comment.
This method is just a wrapper around new keyword. I will remove it.
mikkokar
force-pushed
the
refactor-origin-inventory-builder
branch
from
5a02b80
to
4718907
Compare
Simplify builder class for the OriginsInventory.