From c762c4f3fb7de15537f7c77677be0b79569173d9 Mon Sep 17 00:00:00 2001 From: Andrew Cholewa Date: Mon, 26 Sep 2016 16:55:40 -0500 Subject: [PATCH] Addressed comments. --- CHANGELOG.md | 7 +++++++ .../workflows/DefaultAsynchronousWorkflowsBuilder.java | 6 ++++++ 2 files changed, 13 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08d272e9ea..e88611d1c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,13 @@ Current ### Fixed: +- [Fixes a bug where job metadata was being stored in the `ApiJobStore` even when the results came back synchronously](https://github.com/yahoo/fili/pull/49) + * The workflow that updates the job's metadata with `success` was running even when the query was synchronous. That update had the impact of + storing the ticket in the ApiJobStore. + * The reason the delay operator didn't stop the update workflow from executing viewed an `Observable::onCompleted` as a message for the purpose of + delays. Since the two observables that that the metadata update it gated on are empty when the query is synchronous, the update metadata workflow + was being updated. + * The delay operator was replaced by `zipWith` as a gate. - [#45, removing sorting from weight check queries](https://github.com/yahoo/fili/pull/46) diff --git a/fili-core/src/main/java/com/yahoo/bard/webservice/async/workflows/DefaultAsynchronousWorkflowsBuilder.java b/fili-core/src/main/java/com/yahoo/bard/webservice/async/workflows/DefaultAsynchronousWorkflowsBuilder.java index 79c29efad1..b16f5f9876 100644 --- a/fili-core/src/main/java/com/yahoo/bard/webservice/async/workflows/DefaultAsynchronousWorkflowsBuilder.java +++ b/fili-core/src/main/java/com/yahoo/bard/webservice/async/workflows/DefaultAsynchronousWorkflowsBuilder.java @@ -176,6 +176,8 @@ private Observable buildStorePreResponseChain( // We don't want to store the result in the PreResponseStore unless the query is asynchronous. The query is // asynchronous iff the asynchronousPayload emits at least one item. return preResponseEmitter + // Using zip as a gate. We don't let the preResponse from the preResponse emitter continue down the + // chain until and unless the asynchronousPayload emits an item. .zipWith(asynchronousPayload, (preResponse, ignored) -> preResponse) .flatMap(preResponse -> preResponseStore.save(jobRow.getId(), preResponse)); } @@ -206,7 +208,11 @@ private Observable buildUpdateRowChain( // The job status should not be updated until both the PreResponse has been stored, and the storage of the // original Job ticket has been attempted. return preResponseEmitter + // Using zip as a gate. We don't let the preResponse from the preResponse emitter continue down the + // chain until and unless the jobRowStoredNotification emits an item. .zipWith(jobRowStoredNotification, (preResponse, ignored) -> preResponse) + // Using zip as a gate. We don't let the preResponse from the preResponse emitter continue down the + // chain until and unless the preResponseStoredNotification emits an item. .zipWith(preResponseStoredNotification, (preResponse, ignored) -> preResponse) .map(PreResponse::getResponseContext) .map(responseContext -> responseContext.containsKey(ResponseContextKeys.ERROR_MESSAGE.getName()))