sashapolo commented on code in PR #3240:
URL: https://github.com/apache/ignite-3/pull/3240#discussion_r1496352043
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexAvailabilityController.java:
##########
@@ -189,27 +188,15 @@ public void recover(long recoveryRevision) {
}
private void addListeners(CatalogService catalogService,
MetaStorageManager metaStorageManager, IndexBuilder indexBuilder) {
- catalogService.listen(CatalogEvent.INDEX_BUILDING, (parameters,
exception) -> {
- if (exception != null) {
- return failedFuture(exception);
- }
-
+ catalogService.listen(CatalogEvent.INDEX_BUILDING, parameters -> {
Review Comment:
Same stuff about explicit types
##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java:
##########
@@ -2218,17 +2216,13 @@ void testAvailableIndexEvent() {
CompletableFuture<Void> fireEventFuture = new CompletableFuture<>();
- manager.listen(CatalogEvent.INDEX_AVAILABLE, (parameters, exception)
-> {
- if (exception != null) {
- fireEventFuture.completeExceptionally(exception);
- } else {
- try {
- assertEquals(indexId, ((MakeIndexAvailableEventParameters)
parameters).indexId());
+ manager.listen(CatalogEvent.INDEX_AVAILABLE, parameters -> {
+ try {
+ assertEquals(indexId, ((MakeIndexAvailableEventParameters)
parameters).indexId());
Review Comment:
I would suggest to use the following notation in order to avoid a cast:
```
manager.listen(CatalogEvent.INDEX_AVAILABLE,
(MakeIndexAvailableEventParameters parameters) -> {
try {
assertEquals(indexId, parameters.indexId());
```
##########
modules/core/src/test/java/org/apache/ignite/internal/event/EventProducerTest.java:
##########
@@ -111,12 +108,6 @@ public void parallelTest() {
assertThat(fireFuture, willCompleteSuccessfully());
}
- private static <T extends EventParameters> EventListener<T>
createEventListener(
Review Comment:
oh god
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexManager.java:
##########
@@ -140,13 +140,7 @@ public CompletableFuture<Void> start() {
startIndexes();
- catalogService.listen(INDEX_CREATE, (parameters, exception) -> {
- if (exception != null) {
- return failedFuture(exception);
- }
-
- return onIndexCreate((CreateIndexEventParameters) parameters);
- });
+ catalogService.listen(INDEX_CREATE, parameters ->
onIndexCreate((CreateIndexEventParameters) parameters));
Review Comment:
And here
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexBuildController.java:
##########
@@ -123,27 +122,15 @@ public void close() {
}
private void addListeners() {
- catalogService.listen(CatalogEvent.INDEX_BUILDING, (parameters,
exception) -> {
- if (exception != null) {
- return failedFuture(exception);
- }
-
+ catalogService.listen(CatalogEvent.INDEX_BUILDING, parameters -> {
Review Comment:
And here
##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java:
##########
@@ -2248,15 +2242,11 @@ void testAvailableIndexEvent() {
void testPkAvailableIndexEvent() {
CompletableFuture<Integer> fireEventFuture = new CompletableFuture<>();
- manager.listen(CatalogEvent.INDEX_AVAILABLE, (parameters, exception)
-> {
- if (exception != null) {
- fireEventFuture.completeExceptionally(exception);
- } else {
- try {
-
fireEventFuture.complete(((MakeIndexAvailableEventParameters)
parameters).indexId());
- } catch (Throwable t) {
- fireEventFuture.completeExceptionally(t);
- }
+ manager.listen(CatalogEvent.INDEX_AVAILABLE, parameters -> {
+ try {
+ fireEventFuture.complete(((MakeIndexAvailableEventParameters)
parameters).indexId());
Review Comment:
Same here and in similar places. Also probably this stuff should be
extracted into a method, I can see that it is copy pasted multiple times across
the class
##########
modules/core/src/main/java/org/apache/ignite/internal/event/AbstractEventProducer.java:
##########
@@ -87,7 +87,7 @@ protected CompletableFuture<Void> fireEvent(T evt, P params,
@Nullable Throwable
for (int i = 0; i < listeners.size(); i++) {
EventListener<P> listener = listeners.get(i);
- CompletableFuture<Boolean> future = listener.notify(params, err);
+ CompletableFuture<Boolean> future = listener.notify(params);
Review Comment:
Looks like we no longer need the `err` parameter in this method
##########
modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java:
##########
@@ -1378,17 +1378,13 @@ public Set<NodeWithAttributes> logicalTopology() {
}
private void registerCatalogEventListenersOnStartManagerBusy() {
- catalogManager.listen(ZONE_CREATE, (parameters, exception) ->
inBusyLock(busyLock, () -> {
- assert exception == null : parameters;
-
+ catalogManager.listen(ZONE_CREATE, parameters -> inBusyLock(busyLock,
() -> {
Review Comment:
Same thing: we can use an explicit parameter type and make this code even
shorter
--
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]
