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

websocket variant of query endpoint #3936

Merged
merged 69 commits into from
Jan 15, 2020
Merged

Conversation

S11001001
Copy link
Contributor

@S11001001 S11001001 commented Jan 1, 2020

Fixes #3880, fixes #3878. Obsoletes #3912 (by incorporation of most of its parts) for the purposes of the JSON API itself.

Major changes versus #3912 :

  1. Input interface matches /contracts/search exactly. That means no empty queries are allowed, and the filtering part is actually used.
  2. Endpoint renamed /contracts/searchForever to emphasize that this is not a stream of transactions, as discussed in Add streaming service for queries #3878.
  3. Output objects are of the format {errors, created, archived}, where all members are lists, rather than containing one contract per output object.
CHANGELOG_BEGIN

- [JSON API - Experimental] Websocket contract search at ``/contracts/searchForever``.
  See `issue #3936 <https://github.com/digital-asset/daml/pull/3936>`_.

CHANGELOG_END
  • detach WebsocketServiceIntegrationTest from tests added by AbstractHttpServiceIntegrationTest
  • ensure that nontermination is happening
  • longer test: create, check, exercise, check adds/removes
  • include output in documentation

Pull Request Checklist

  • Read and understand the contribution guidelines
  • Include appropriate tests
  • Set a descriptive title and thorough description
  • Add a reference to the issue this PR will solve, if appropriate
  • Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags, if appropriate
  • Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

@S11001001 S11001001 added the component/json-api HTTP JSON API label Jan 1, 2020
@S11001001 S11001001 added this to the HTTP JSON API Maintenance milestone Jan 1, 2020
@S11001001 S11001001 self-assigned this Jan 1, 2020
- clients may not be in control of how query bodies are delivered to the
  server, so we should be agnostic in that respect
CHANGELOG_BEGIN

- [JSON API - Experimental] WebSocket contract search at ``/contracts/searchForever``.
  See `issue #3936 <https://github.com/digital-asset/daml/pull/3936>`_.

CHANGELOG_END
@digitalasset-cla
Copy link
Member

digitalasset-cla commented Jan 15, 2020

CLA assistant check
All committers have signed the CLA.

@S11001001 S11001001 marked this pull request as ready for review January 15, 2020 21:09
@S11001001 S11001001 requested review from leo-da and lima-da January 15, 2020 21:10
Copy link
Contributor

@leo-da leo-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM;

docs/source/json-api/index.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@lima-da lima-da left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me too!

@S11001001 S11001001 merged commit 3ae0438 into master Jan 15, 2020
@S11001001 S11001001 deleted the 3880-websocket-acs-graph branch January 15, 2020 23:11
After submitting an ``Iou_Split`` exercise, which creates two contracts
and archives the one above, the same stream will eventually produce::

{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This format differs from what you get from the /commands/exercise endpoint. Her you get something looking like

{
  created: [C1, C2],
  archived: [A1, A2]
}

There you'd instead get

[
  {created: C1},
  {created: C2},
  {archived: A1},
  {archived: A2}
]

I'd prefer if the encoding is the same for both endpoints. Since the /commands/exercise endpoint does not work with the first format, we need to use the second in both places. It would be nice if could keep the order of the events we don't filter out during consolidation for the /contracts/searchForever endpoint, but I don't consider it a hard requirement.

@stefanobaghino-da stefanobaghino-da added the team/ledger-clients Related to the Ledger Clients team's components. label Aug 31, 2021
@S11001001 S11001001 restored the 3880-websocket-acs-graph branch March 8, 2022 20:01
@stephencompall-DA stephencompall-DA deleted the 3880-websocket-acs-graph branch March 8, 2022 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/json-api HTTP JSON API team/ledger-clients Related to the Ledger Clients team's components.
Projects
None yet
6 participants