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

completionStream rpc rejects requests without an offset #1913

Closed
nickchapman-da opened this issue Jun 27, 2019 · 1 comment

Comments

Projects
None yet
2 participants
@nickchapman-da
Copy link
Contributor

commented Jun 27, 2019

When no offset is provided in a CompletionStreamRequest, the comments states:

  // This field indicates the minimum offset for completions. This can be used to resume an earlier completion stream.
  // Optional, if not set the ledger uses the current ledger end offset instead.
  LedgerOffset offset = 4;

I would have expected to get stream which tails future completions, but instead observe a stream which is immediately closed.

@stefanobaghino-da stefanobaghino-da self-assigned this Jun 27, 2019

@stefanobaghino-da stefanobaghino-da changed the title completionStream rpc returns empty stream completionStream rpc rejects requests without an offset Jul 1, 2019

@stefanobaghino-da

This comment has been minimized.

Copy link
Member

commented Jul 1, 2019

After investigation, the actual issue is that the stream closes immediately with an error because the offset parameter is actually required by the sandbox.

stefanobaghino-da added a commit that referenced this issue Jul 1, 2019

The completion stream RPC defaults to the ledger end as offset
Fixes #1913

Relevant changes are propagated to the Java bindings (including
deprecating a method that would now return a nullable ledger end).

stefanobaghino-da added a commit that referenced this issue Jul 1, 2019

The completion stream RPC defaults to the ledger end as offset
Fixes #1913

Relevant changes are propagated to the Java bindings (including
deprecating a method that would now return a nullable ledger end).

stefanobaghino-da added a commit that referenced this issue Jul 1, 2019

The completion stream RPC defaults to the ledger end as offset
Fixes #1913

Relevant changes are propagated to the Java bindings (including
deprecating a method that would now return a nullable ledger end).

@mergify mergify bot closed this in #1961 Jul 2, 2019

mergify bot added a commit that referenced this issue Jul 2, 2019

The completion stream RPC defaults to the ledger end as offset (#1961)
* The completion stream RPC defaults to the ledger end as offset

Fixes #1913

Relevant changes are propagated to the Java bindings (including
deprecating a method that would now return a nullable ledger end).

* Refactor completionStream method

* Address review comments

- ignore command creation results
(#1961 (comment))
- avoid re-connecting to the client for every command
(#1961 (comment))
- move offset field optionality to domain object
(#1961 (comment))

* Improve tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.