Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Javascript driver makes irritating assumptions about batched streams #1544

Closed
mlucy opened this issue Oct 15, 2013 · 6 comments
Closed

Javascript driver makes irritating assumptions about batched streams #1544

mlucy opened this issue Oct 15, 2013 · 6 comments
Assignees
Milestone

Comments

@mlucy
Copy link
Member

mlucy commented Oct 15, 2013

Currently, the javascript driver assumes that if it gets back SUCCESS_PARTIAL, there is more data to be read. That is, it can't handle a SUCCESS_PARTIAL followed by an empty SUCCESS_STREAM.

The server currently supports this assumption by holding on to at least one piece of data before sending back SUCCESS_PARTIAL. It would be nice if we didn't have to do this.

@neumino
Copy link
Member

neumino commented Oct 15, 2013

If I'm not mistaken, the reason why the server has ton compute one piece of data before sending SUCCESS_PARTIAL is because of hasNext.

Are you suggesting that hasNext should be more javascript-like -- that is to say asynchronous?
(or get rid of it?)

@mlucy
Copy link
Member Author

mlucy commented Oct 16, 2013

The javascript driver can hold on to the one result rather than expecting the server to. It doesn't give the user the last result of a SUCCESS_PARTIAL until it has the next response from the server.

@srh
Copy link
Contributor

srh commented Oct 17, 2013

Note that it should be a different protocol buffer command that implements these new semantics -- the existing one that gets these SUCCESS_PARTIAL and SUCCESS_STREAM responses should be marked obsolete but still supported for now to save our users some pain when upgrading.

@ghost ghost assigned Tryneus Oct 23, 2013
@coffeemug
Copy link
Contributor

Assigning to @Tryneus.

@Tryneus
Copy link
Member

Tryneus commented Oct 31, 2013

I have a fix up for the js driver in review 1001. This also includes a change for the server to no longer ensure that SUCCESS_SEQUENCE contains at least one row (this simplifies the logic in stream_cache2_t significantly).

@Tryneus
Copy link
Member

Tryneus commented Oct 31, 2013

This has been approved and merged into next in commits 56d5d6b and d0b6521. Will be in release 1.11.

@Tryneus Tryneus closed this as completed Oct 31, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants