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

Multiplex multiple virtual query streams through a single web socket #10221

Merged
merged 24 commits into from Oct 29, 2021

Conversation

stefanobaghino-da
Copy link
Contributor

@stefanobaghino-da stefanobaghino-da commented Jul 8, 2021

Fixes #7796

changelog_begin
[TS bindings] When using daml-react and daml-ledger, all streaming request to the query endpoint will be multiplexed through a single web socket
changelog_end

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
  • 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.

stefanobaghino-da added a commit that referenced this pull request Jul 15, 2021
stefanobaghino-da added a commit that referenced this pull request Jul 15, 2021
stefanobaghino-da added a commit that referenced this pull request Jul 15, 2021
stefanobaghino-da added a commit that referenced this pull request Jul 15, 2021
stefanobaghino-da added a commit that referenced this pull request Sep 15, 2021
stefanobaghino-da added a commit that referenced this pull request Sep 15, 2021
stefanobaghino-da added a commit that referenced this pull request Sep 15, 2021
stefanobaghino-da added a commit that referenced this pull request Sep 15, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 13, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 13, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 13, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 13, 2021
@akshayshirahatti-da akshayshirahatti-da self-assigned this Oct 25, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 26, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 26, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 26, 2021
akshayshirahatti-da pushed a commit that referenced this pull request Oct 26, 2021
@akshayshirahatti-da akshayshirahatti-da marked this pull request as ready for review October 26, 2021 17:39
Copy link
Contributor Author

@stefanobaghino-da stefanobaghino-da left a comment

Choose a reason for hiding this comment

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

Mostly nitpicking and a couple of doubts. Overall LGTM. If you can, I would recommend to allow users to disable multiplexing (as an escape hatch in case we missed a bug).

language-support/ts/daml-ledger/index.test.ts Outdated Show resolved Hide resolved
language-support/ts/daml-ledger/index.test.ts Show resolved Hide resolved
language-support/ts/daml-ledger/index.test.ts Show resolved Hide resolved
language-support/ts/daml-ledger/index.test.ts Outdated Show resolved Hide resolved
Comment on lines +152 to +153
page.on('console', message =>
console.log(`${message.type().substr(0, 3).toUpperCase()} ${message.text()}`))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this a leftover from a debugging session?

Copy link
Contributor

Choose a reason for hiding this comment

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

it is , I left it there since it was a bit frustrating to not have any logs about puppeteer browser interactions. I think there is value to have them and they shouldn't be that large in size.

language-support/ts/daml-ledger/index.ts Outdated Show resolved Hide resolved
language-support/ts/daml-ledger/index.ts Outdated Show resolved Hide resolved
Co-authored-by: Stefano Baghino <43749967+stefanobaghino-da@users.noreply.github.com>
Co-authored-by: Stefano Baghino <43749967+stefanobaghino-da@users.noreply.github.com>
Co-authored-by: Stefano Baghino <43749967+stefanobaghino-da@users.noreply.github.com>
Co-authored-by: Stefano Baghino <43749967+stefanobaghino-da@users.noreply.github.com>
@akshayshirahatti-da
Copy link
Contributor

Mostly nitpicking and a couple of doubts. Overall LGTM. If you can, I would recommend to allow users to disable multiplexing (as an escape hatch in case we missed a bug).

thats a good shout , I am not sure how feature flags are done with typescript , you have any pointers for that @stefanobaghino-da ?

@stefanobaghino-da
Copy link
Contributor Author

Mostly nitpicking and a couple of doubts. Overall LGTM. If you can, I would recommend to allow users to disable multiplexing (as an escape hatch in case we missed a bug).

thats a good shout , I am not sure how feature flags are done with typescript , you have any pointers for that @stefanobaghino-da ?

To me this would be a boolean flag that the user can set when using the bindings. Do we not have any point where the user calls a function with a parameter?

@@ -66,11 +78,15 @@ jest.mock('isomorphic-ws', () => class {

serverClose(event: {code: number; reason: string}): void {
this.eventEmitter.emit('close', event);
// eslint-disable-next-line @typescript-eslint/no-var-requires
const {WsState} = require('./index');
Copy link
Contributor

Choose a reason for hiding this comment

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

don't like this repetitive code, but its in the test and I couldn't find an easier way to get around jestjs/jest#2567

Copy link
Contributor

Choose a reason for hiding this comment

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

You could probably move the WsState definition one level up and make it a class-level attribute, which would save a little bit of repetition.

@akshayshirahatti-da
Copy link
Contributor

To me this would be a boolean flag that the user can set when using the bindings. Do we not have any point where the user calls a function with a parameter?

Makes sense I have added a param to the LedgerOptions for this

  /**
   * Optional to enable/disable feature of all streaming request to the query endpoint being multiplexed
   * through a single web socket, is enabled by default.
   */
  multiplexQueryStreams?: boolean;

@akshayshirahatti-da akshayshirahatti-da merged commit 05505e3 into main Oct 29, 2021
@akshayshirahatti-da akshayshirahatti-da deleted the ste/7796 branch October 29, 2021 08:20
azure-pipelines bot pushed a commit that referenced this pull request Nov 3, 2021
This PR has been created by a script, which is not very smart
and does not have all the context. Please do double-check that
the version prefix is correct before merging.

@robin-da is in charge of this release.

Commit log:
```
7391a3c Migrate language-support codegen tests to sandbox (#11508)
da76e28 iface: check for interface implementations precond (#11494)
9e045c1 Remove a leftover from `check-protobuf-stability.sh` (#11513)
17c2fa9 ErrorCodeDocumentationGenerator uses multiple prefixes for scanning classpaths (#11504)
a8ae4d6 LF: Rename GenNode to Node (#11469)
73b6f8b Fixed ledger-api-bench-tool random distribution in submission phase. (#11507)
92495b0 Upgrade nixpkgs (#11490)
48afb7b interface: Add inherited choice names in scala AST (#11503)
9f8b039 [Trigger-Service] Migrate tests to sandbox (#11501)
6bafeb0 DPP-662 Support archivals in bench tool (#11474)
63d855f Pass `damlc build` logs through daml logger (#11485)
565dfac Bump rules_nodejs to 4.4.2 (#11495)
9214956 Enable experimental nix command (#11498)
f81d880 LF: drop com.daml.lf.transaction.Transaction.Transaction type alias. (#11470)
505a7f5 Extend debug logging around dependencies/data-dependencies (#11488)
55d35e5 Drop TLA+ from dev-env (#11492)
3a5cb18 [Self-service error codes] Adapt all grpc assertions to additionally take self-service error codes [DPP-680] (#11437)
98e7461 LF: ContractInstance should be a CidContainer (#11487)
9afb3b3 Add and wire ledger-end-cache [DPP-703] (#11480)
cdd2acc Explicitly import Java runtime from Nix/dadew (#11411)
b5540a9 Evidence for contract-key testing (#11479)
78735a8 Less logging on //docs:generate-error-codes-json (#11486)
d8898b8 Check fixed choices in name collision checker (#11481)
7a0198b daml-react: allow for React 17 (#11463)
c415d9d Migrate trigger tests from sandbox-classic to sandbox (#11478)
f9ec12b Add cocreature as a codeowner of the docs (#11450)
a79cc08 DbDispatcher handles SQL exceptions with self-service error codes [DPP-690] (#11435)
164cdbb update compat versions for 1.18.0-snapshot.20211026.8179.0.e474b2d1 (#11426)
741a6e7 interfaces: Add list of fixed choices in TemplateImplements structure. (#11364)
79505b5 oracle debug (#11386)
257d536 Switch from sandbox to sandbox-classic in daml test-script (#11476)
e91ced5 KV: generate self-service error docs [KVL-1148] (#11431)
8ffd835 Migrate daml script tests to Sandbox next (#11472)
df65d02 Exclude dedup tests affected by current changes (#11471)
45beaf7 interfaces: typecheck for precond, haskell side. (#11468)
e445fe1 LfEngineToApi conversions do not have to deal with version. (#11414)
d371274 Sandbox-classic: contracts sent to the engine do not need enrichment (#11416)
05312f1 Lift out constraint tuples to synonyms in dfun contexts (#11467)
a492a6c [DPP-636] Update docs on MacOS firewall issue when running tests (#11460)
5a85881 rotate release duty after 1.18.0-snapshot.20211026.8179.0.e474b2d1 (#11422)
1096f91 Adds default keepalive support for postgreSQL [DPP-697] (#11453)
3799963 Fix akka-streams shutdown at LedgerConfigurationSubscriptionFromIndex (#11462)
e89da7c Move non-repudiation proxy conformance tests to their own package (#11464)
b98a3ad interfaces: precondition, scala ast, decoder/encoder (#11452)
fd973e6 Drop assistant integration tests for relative/absolute DAML_PROJECT (#11461)
05505e3 Multiplex multiple virtual query streams through a single web socket (#10221)
3098b70 pretty print contract IDs properly (#11359)
bb4f4c5 DPP-665 Make in-flight commands configurable (#11456)
bf00956 replace OneAnd party Sets with NonEmpty Set (#11420)
570160b Add LoggingValue instance for Time.Timestamp (#11444)
bad50a5 Type reexports explicitly use 'type' namespace (#11451)
d3b9c84 ifaces: preconditions, proto changes (#11448)
73718a2 Mark script export tests as flaky (#11449)
22e9e6b Multi-observer command submission in the ledger-api-bench-tool [DPP-660] (#11413)
3eacbf9 Bazel cache retry on connection reset by peer (#11445)
fcc7e58 Make stable packages dependent on supported LF versions (#11382)
6c45f09 Remove TODO (#11446)
d8b92b7 Drop flaky marker from lsp-tests (#11442)
68f4432 Improve evidence of testing (#11428)
6126fc2 Remove append only prefix from conformance tests [KVL-1152] (#11424)
926655d Drop daml ledger timeout tests from compat tests (#11441)
15cb840 Switch back to binary execution logs (#11440)
ac28e61 Update ScalaPB to the latest version (0.11.6) (#11409)
a71a8f9 Prevent overflow in DeduplicationPeriod (#11432)
6c88e55 Pass submission id as correlation id to error codes in Ledger API (#11381)
ba10687 Replace 1.14 min version conformance test by custom test (#11433)
a600f75 Bump bazel memory limit to 3gb (#11436)
ebe7420 fix es ingest: file too large (#11415)
d86eca0 Add documentation annotations to KV self-service errors [KVL-1144] (#11385)
46d31d5 [DPP-676][Self-service error codes] Adapt ApiConfigManagementService (#11402)
927b237 Make Oracle tests exclusive (#11408)
68d7f83 Switch bazel exec logs to json format (#11430)
12e782b ifaces: update ghc-parser (#11383)
503e391 [Short] Typo (#11410)
3031190 [DPP-676][Self-service error codes] Adapt request validators (#11379)
b412f71 Fix MonotonicRecordTimeIT for pruned participants (#11423)
500e5b0 release 1.18.0-snapshot.20211026.8179.0.e474b2d1 (#11421)
d678a40 LF: remove type parameter from ContractInstance (#11419)
```
Changelog:
```

- [@daml/react] Our React bindings now request the `react` library as a
  peer dependency, and will accept React 17 in addition to the current
  React 16. For new projects, the `create-daml-app` template now uses
  React 17.

- [Integration Kit] Document KV-specific self-service errors

[TS bindings] When using daml-react and daml-ledger, all streaming request to the query endpoint will be multiplexed through a single web socket.
[daml LF] - Add LoggingValue instance for Time.Timestamp
- [Integration Kit] - ledger-api-bench-tool can generate synthetic contracts with multiple observers
[api-test-tool] - remove AppendOnly prefix from test suites as the append-only schema is the only one left
```

CHANGELOG_BEGIN
CHANGELOG_END
garyverhaegen-da pushed a commit that referenced this pull request Nov 3, 2021
This PR has been created by a script, which is not very smart
and does not have all the context. Please do double-check that
the version prefix is correct before merging.

@robin-da is in charge of this release.

Commit log:
```
7391a3c Migrate language-support codegen tests to sandbox (#11508)
da76e28 iface: check for interface implementations precond (#11494)
9e045c1 Remove a leftover from `check-protobuf-stability.sh` (#11513)
17c2fa9 ErrorCodeDocumentationGenerator uses multiple prefixes for scanning classpaths (#11504)
a8ae4d6 LF: Rename GenNode to Node (#11469)
73b6f8b Fixed ledger-api-bench-tool random distribution in submission phase. (#11507)
92495b0 Upgrade nixpkgs (#11490)
48afb7b interface: Add inherited choice names in scala AST (#11503)
9f8b039 [Trigger-Service] Migrate tests to sandbox (#11501)
6bafeb0 DPP-662 Support archivals in bench tool (#11474)
63d855f Pass `damlc build` logs through daml logger (#11485)
565dfac Bump rules_nodejs to 4.4.2 (#11495)
9214956 Enable experimental nix command (#11498)
f81d880 LF: drop com.daml.lf.transaction.Transaction.Transaction type alias. (#11470)
505a7f5 Extend debug logging around dependencies/data-dependencies (#11488)
55d35e5 Drop TLA+ from dev-env (#11492)
3a5cb18 [Self-service error codes] Adapt all grpc assertions to additionally take self-service error codes [DPP-680] (#11437)
98e7461 LF: ContractInstance should be a CidContainer (#11487)
9afb3b3 Add and wire ledger-end-cache [DPP-703] (#11480)
cdd2acc Explicitly import Java runtime from Nix/dadew (#11411)
b5540a9 Evidence for contract-key testing (#11479)
78735a8 Less logging on //docs:generate-error-codes-json (#11486)
d8898b8 Check fixed choices in name collision checker (#11481)
7a0198b daml-react: allow for React 17 (#11463)
c415d9d Migrate trigger tests from sandbox-classic to sandbox (#11478)
f9ec12b Add cocreature as a codeowner of the docs (#11450)
a79cc08 DbDispatcher handles SQL exceptions with self-service error codes [DPP-690] (#11435)
164cdbb update compat versions for 1.18.0-snapshot.20211026.8179.0.e474b2d1 (#11426)
741a6e7 interfaces: Add list of fixed choices in TemplateImplements structure. (#11364)
79505b5 oracle debug (#11386)
257d536 Switch from sandbox to sandbox-classic in daml test-script (#11476)
e91ced5 KV: generate self-service error docs [KVL-1148] (#11431)
8ffd835 Migrate daml script tests to Sandbox next (#11472)
df65d02 Exclude dedup tests affected by current changes (#11471)
45beaf7 interfaces: typecheck for precond, haskell side. (#11468)
e445fe1 LfEngineToApi conversions do not have to deal with version. (#11414)
d371274 Sandbox-classic: contracts sent to the engine do not need enrichment (#11416)
05312f1 Lift out constraint tuples to synonyms in dfun contexts (#11467)
a492a6c [DPP-636] Update docs on MacOS firewall issue when running tests (#11460)
5a85881 rotate release duty after 1.18.0-snapshot.20211026.8179.0.e474b2d1 (#11422)
1096f91 Adds default keepalive support for postgreSQL [DPP-697] (#11453)
3799963 Fix akka-streams shutdown at LedgerConfigurationSubscriptionFromIndex (#11462)
e89da7c Move non-repudiation proxy conformance tests to their own package (#11464)
b98a3ad interfaces: precondition, scala ast, decoder/encoder (#11452)
fd973e6 Drop assistant integration tests for relative/absolute DAML_PROJECT (#11461)
05505e3 Multiplex multiple virtual query streams through a single web socket (#10221)
3098b70 pretty print contract IDs properly (#11359)
bb4f4c5 DPP-665 Make in-flight commands configurable (#11456)
bf00956 replace OneAnd party Sets with NonEmpty Set (#11420)
570160b Add LoggingValue instance for Time.Timestamp (#11444)
bad50a5 Type reexports explicitly use 'type' namespace (#11451)
d3b9c84 ifaces: preconditions, proto changes (#11448)
73718a2 Mark script export tests as flaky (#11449)
22e9e6b Multi-observer command submission in the ledger-api-bench-tool [DPP-660] (#11413)
3eacbf9 Bazel cache retry on connection reset by peer (#11445)
fcc7e58 Make stable packages dependent on supported LF versions (#11382)
6c45f09 Remove TODO (#11446)
d8b92b7 Drop flaky marker from lsp-tests (#11442)
68f4432 Improve evidence of testing (#11428)
6126fc2 Remove append only prefix from conformance tests [KVL-1152] (#11424)
926655d Drop daml ledger timeout tests from compat tests (#11441)
15cb840 Switch back to binary execution logs (#11440)
ac28e61 Update ScalaPB to the latest version (0.11.6) (#11409)
a71a8f9 Prevent overflow in DeduplicationPeriod (#11432)
6c88e55 Pass submission id as correlation id to error codes in Ledger API (#11381)
ba10687 Replace 1.14 min version conformance test by custom test (#11433)
a600f75 Bump bazel memory limit to 3gb (#11436)
ebe7420 fix es ingest: file too large (#11415)
d86eca0 Add documentation annotations to KV self-service errors [KVL-1144] (#11385)
46d31d5 [DPP-676][Self-service error codes] Adapt ApiConfigManagementService (#11402)
927b237 Make Oracle tests exclusive (#11408)
68d7f83 Switch bazel exec logs to json format (#11430)
12e782b ifaces: update ghc-parser (#11383)
503e391 [Short] Typo (#11410)
3031190 [DPP-676][Self-service error codes] Adapt request validators (#11379)
b412f71 Fix MonotonicRecordTimeIT for pruned participants (#11423)
500e5b0 release 1.18.0-snapshot.20211026.8179.0.e474b2d1 (#11421)
d678a40 LF: remove type parameter from ContractInstance (#11419)
```
Changelog:
```

- [@daml/react] Our React bindings now request the `react` library as a
  peer dependency, and will accept React 17 in addition to the current
  React 16. For new projects, the `create-daml-app` template now uses
  React 17.

- [Integration Kit] Document KV-specific self-service errors

[TS bindings] When using daml-react and daml-ledger, all streaming request to the query endpoint will be multiplexed through a single web socket.
[daml LF] - Add LoggingValue instance for Time.Timestamp
- [Integration Kit] - ledger-api-bench-tool can generate synthetic contracts with multiple observers
[api-test-tool] - remove AppendOnly prefix from test suites as the append-only schema is the only one left
```

CHANGELOG_BEGIN
CHANGELOG_END

Co-authored-by: Azure Pipelines Daml Build <support@digitalasset.com>
@S11001001 S11001001 added component/json-api HTTP JSON API team/ledger-clients Related to the Ledger Clients team's components. labels Mar 14, 2022
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
Development

Successfully merging this pull request may close these issues.

Reduce the number of transaction streams resulting from JS stream queries
5 participants