Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public final class CentralDogmaEndpointGroup<T> extends DynamicEndpointGroup {
public static <T> CentralDogmaEndpointGroup<T> ofWatcher(Watcher<T> watcher,
EndpointListDecoder<T> endpointListDecoder) {
return new CentralDogmaEndpointGroup<>(EndpointSelectionStrategy.weightedRoundRobin(),
watcher, endpointListDecoder);
watcher, endpointListDecoder, false);
}

/**
Expand Down Expand Up @@ -107,13 +107,30 @@ public static <T> CentralDogmaEndpointGroup<T> of(CentralDogma centralDogma,
*/
public static <T> CentralDogmaEndpointGroupBuilder<T> builder(Watcher<T> watcher,
EndpointListDecoder<T> endpointListDecoder) {
return new CentralDogmaEndpointGroupBuilder<>(watcher, endpointListDecoder);
return new CentralDogmaEndpointGroupBuilder<>(watcher, endpointListDecoder, false);
}

/**
* Returns a new {@link CentralDogmaEndpointGroupBuilder} with the {@link Watcher}
* and {@link EndpointListDecoder}. You can create a {@link Watcher} using {@link CentralDogma}:
*
* <pre>{@code
* CentralDogma centralDogma = ...
* Query<T> query = ... // The query to the entry that contains the list of endpoints.
* Watcher watcher = centralDogma.fileWatcher(projectName, repositoryName, query);
* }</pre>
*/
public static <T> CentralDogmaEndpointGroupBuilder<T> builder(Watcher<T> watcher,
EndpointListDecoder<T> endpointListDecoder,
boolean allowEmptyEndpoints) {
Comment thread
m50d marked this conversation as resolved.
Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional) Given that we're already using a builder pattern and allowEmptyEndpoints's default value is reasonable, I wonder if it's a better idea to add a builder method instead of adding to the builder factory method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use sites I've seen don't really use it as a builder, they all call the factory method and build() immediately, so this seemed more in line with existing use? But happy to follow the builder style if you prefer.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My line of thought is just from an API management point - each param could possibly add 2^n overload variants.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Doing it that way.

return new CentralDogmaEndpointGroupBuilder<>(watcher, endpointListDecoder, allowEmptyEndpoints);
}

CentralDogmaEndpointGroup(EndpointSelectionStrategy strategy,
Watcher<T> instanceListWatcher,
EndpointListDecoder<T> endpointListDecoder) {
super(strategy);
EndpointListDecoder<T> endpointListDecoder,
boolean allowEmptyEndpoints) {
super(strategy, allowEmptyEndpoints);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
this.instanceListWatcher = requireNonNull(instanceListWatcher, "instanceListWatcher");
this.endpointListDecoder = requireNonNull(endpointListDecoder, "endpointListDecoder");
registerWatcher();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ public final class CentralDogmaEndpointGroupBuilder<T> {

private final Watcher<T> watcher;
private final EndpointListDecoder<T> endpointListDecoder;
private final boolean allowEmptyEndpoints;
private EndpointSelectionStrategy selectionStrategy = EndpointSelectionStrategy.weightedRoundRobin();

CentralDogmaEndpointGroupBuilder(Watcher<T> watcher, EndpointListDecoder<T> endpointListDecoder) {
CentralDogmaEndpointGroupBuilder(Watcher<T> watcher, EndpointListDecoder<T> endpointListDecoder, boolean allowEmptyEndpoints) {
this.watcher = requireNonNull(watcher, "watcher");
this.endpointListDecoder = requireNonNull(endpointListDecoder, "endpointListDecoder");
this.allowEmptyEndpoints = allowEmptyEndpoints;
}

/**
Expand All @@ -52,6 +54,6 @@ public CentralDogmaEndpointGroupBuilder selectionStrategy(EndpointSelectionStrat
* from an entry in Central Dogma.
*/
public CentralDogmaEndpointGroup<T> build() {
return new CentralDogmaEndpointGroup<>(selectionStrategy, watcher, endpointListDecoder);
return new CentralDogmaEndpointGroup<>(selectionStrategy, watcher, endpointListDecoder, allowEmptyEndpoints);
}
}
Loading