Synchronous queries are no longer stored in ApiJobStore. #49
Conversation
archolewa
force-pushed
the
storing-tickets-in-apistore
branch
from
878a367 to
2313131
Compare
| ); | ||
| ) | ||
| .replay(1); | ||
| jobRowUpdatedNotification.connect(); |
There was a problem hiding this comment.
The other notifications are hot, and since this observable relies on hot observables, I felt it was safest to also make it hot.
archolewa
force-pushed
the
storing-tickets-in-apistore
branch
2 times, most recently
from
64a68f4 to
3425245
Compare
|
Needs a CHANGELOG. |
| .delay(ignored -> preResponseStoredNotification) | ||
| .delay(ignored -> jobRowStoredNotification) | ||
| .zipWith(jobRowStoredNotification, (preResponse, ignored) -> preResponse) | ||
| .zipWith(preResponseStoredNotification, (preResponse, ignored) -> preResponse) |
There was a problem hiding this comment.
A comment around why we can ignore the other side of these zips would be good.
There was a problem hiding this comment.
The comment above the stream is supposed to explain it, by explaining that we need to wait until the job row has been stored, and the pre response has been stored before emitting the PreResponse (so we don't care about the value of the notifications, only that they arrived). I can make it more explicit that we're using the zipWith as a gating mechanism if you'd like.
There was a problem hiding this comment.
Yeah, the "zip as gate" is a little odd, I think.
There was a problem hiding this comment.
It is, but I don't really know of a better way. Not if we want to release it iff the two observables emit items.
| jobs: {[ | ||
| //'greg' is the UserId hard-coded in TestBinderFactory::buildJobRowBuilder and used for | ||
| //all queries created in the test environment. | ||
| filters: ["userId-eq[greg]"] |
There was a problem hiding this comment.
Any reason we couldn't just get all of them?
There was a problem hiding this comment.
Because the job store is prepopulated with jobs for functional tests against the jobs endpoint, so it's not empty.
|
After CHANGELOG entry. 👍 |
archolewa
force-pushed
the
storing-tickets-in-apistore
branch
from
c762c4f to
218c6e2
Compare
| 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. |
There was a problem hiding this comment.
This bullet seems... confusing? Could it be cleaned / tightened up a bit?
-- The `delay` operator considers `Observable::onCompleted` to be an "emitted item." Therefore, the job row was still being stored in the ApiJobStore even when the query was synchronous. --This has been fixed by replacing `delay` with a subscription, and/or zip when appropriate.
cdeszaq
force-pushed
the
storing-tickets-in-apistore
branch
from
218c6e2 to
7091fc9
Compare
-- The
delayoperator considersObservable::onCompletedto be an"emitted item." Therefore, the job row was still being stored in the
ApiJobStore even when the query was synchronous.
--This has been fixed by replacing
delaywith a subscription, and/orzip when appropriate.