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.

Sign up for GitHub

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

Jump to bottom

kvutils: Add metadata to Err [KVL-1032] #10992

Merged
merged 9 commits into from
Sep 27, 2021
Merged

kvutils: Add metadata to Err [KVL-1032] #10992

merged 9 commits into from
Sep 27, 2021

Conversation

hubert-da
Copy link
Collaborator

@hubert-da hubert-da commented Sep 22, 2021 •
edited
Loading

A follow-up to:

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.

@hubert-da
kvutils: Add structured error data to the error metadata [KVL-1032]
3ca56ee 
CHANGELOG_BEGIN
CHANGELOG_END
@hubert-da
Address review comments
776511d 
CHANGELOG_BEGIN
CHANGELOG_END
@hubert-da
Cleanup
a02996d 
CHANGELOG_BEGIN
CHANGELOG_END
@hubert-da
kvutils: Add metadata to Err [KVL-1032]
d7c4340 
CHANGELOG_BEGIN
CHANGELOG_END
@hubert-da hubert-da requested review from miklos-da and a user September 22, 2021 15:24
@hubert-da
Add omitted line
91fab44 
CHANGELOG_BEGIN
CHANGELOG_END
@ghost
Copy link

ghost commented Sep 22, 2021

Why is this PR targeting another branch, rather than main?

Can we wait until that other branch is merged to merge this one, or are you expecting that we need to merge this one first?

Base automatically changed from hubert-da/kvutils/structured-error-data to main September 22, 2021 16:49
miklos-da
miklos-da reviewed Sep 23, 2021
View reviewed changes
...ant-state/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/Conversions.scala Outdated
@@ -567,6 +581,7 @@ private[state] object Conversions {
buildStatus(
Code.ABORTED,
s"Inconsistent: Missing input state for key ${rejection.getKey.toString}",
Map("key" -> rejection.getKey.toString),
Copy link
Contributor

Choose a reason for hiding this comment

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

This will produce an output with newlines. Given that the goal of these pieces of metadata is not to print them out on the console, I'd suggest using shortDebugString instead, as that doesn't produce newlines and is still human readable.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry, this is something that has been merged in the previous PR. We decided to leave this one, as it's possible to parse it with: TextFormat.parse, and this is what is demonstrated by a test. Are you ok with that?

@hubert-da
Fix names, add test cases
ae9c5b0 
CHANGELOG_BEGIN
CHANGELOG_END
@hubert-da hubert-da marked this pull request as ready for review September 23, 2021 08:51
@hubert-da hubert-da requested review from meiersi-da and a team as code owners September 23, 2021 08:51
@hubert-da hubert-da enabled auto-merge (squash) September 23, 2021 14:52
@hubert-da hubert-da merged commit ee9be65 into main Sep 27, 2021
@hubert-da hubert-da deleted the hubert-da/kvutils/structured-error-data-followup branch September 27, 2021 11:42
azure-pipelines bot pushed a commit that referenced this pull request Sep 29, 2021
release 1.18.0-snapshot.20210928.7948.0.b4d00317
7e6b019 
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.

@stefanobaghino-da is in charge of this release.

Commit log:
```
b4d0031 detect unsynchronized contract table and retry (#10617)
3d779cf [Mutable state cache] Fix initialization offset (#11024)
5458aa8 Switch on NonUnitStatements warning in daml-lf/transaction (#11048)
9641fd5 auth middleware: no print secret (#11050)
a885f52 [DPP-417] [DPP-595] Add error code version switching mechanism (#11035)
12e0c72 fix blackduck logic (#11049)
eb87b34 kvutils: Add the logging context for ledger state operations. (#11030)
b7daa5f Address remaining dependabot alerts (#11045)
5e43f8c es: drop jobs-* indices (#10857)
03203b7 Define encoding/decoding for module imports (#11036)
57a1597 Setting timeoutToleranceMillis to 10 minutes to prevent flakiness (#11043)
df59f3f Fix Navigator dependabot alerts (#11044)
6bf45a3 Upgrade Navigator to Webpack 5 (#11040)
80e217e [DPP-622] Add conformance tests that verifies TLSv1.0 and TLSv1 are disabled. (#10983)
626e1fb Small lf value.cids regression fix (found by canton unit tests) (#11032)
a4629a4 KV: Ignore daml_lf_1.proto when checking for KV protobuf compatibility (#11021)
ee9be65 kvutils: Add metadata to `Err` [KVL-1032] (#10992)
0d3ae6e interface methods: Haskell Typechecker (#11028)
e36eb46 Resolve `set-value` to 4.0.1 and above (#11029)
4075624 interface methods: Haskell AST (#11018)
7c1fd50 Bump grunt-browserify (#11026)
5f3f582 Upgrade webpack-dev-server in Navigator (#11025)
91be1e1 Drop matchdep dependency from docs build (#11023)
fe9aeff Increase es disk size (#11019)
eac7963 LF: Refactor ProtoTest.scala (#11020)
e79a30a For the client binding propagate the full original completion [KVL-1112] (#10879)
59ad995 fix buf check (#11014)
abc3e66 Increase the tolarance for handover of control in HaCoordinatorSpec (#10997)
19b2bf4 LF: Cosmetic clean-up in the Speedy Compiler (#11015)
cb0e41f LF: Add interface support to the Preprocessor (#11013)
c33297c Remove `transactionNormalization` flag. (#11010)
f5d2135 Check protobuf compatibility of `main` and PR commits w.r.t. previous stable release [KVL-1109] (#10950)
bf8b75d interface methods: Add protobuf definitions. (#11005)
88e1430 Make LargeTransactionTest use ValueEnricher, so it can work with normalized transactions coming out of the engine. (#11003)
a043926 rotate release duty after 1.17.0-snapshot.20210921.7889.0.1b473c2b (#10972)
35666ca Add MINIMAL pragma for Additive type class (#11001)
8de162b [DPP-586] Upgrade to netty 4.1.67.Final and netty-tcnative-boringssl-static 2.0.40.Final (#10956)
871d03b release 1.18.0-snapshot.20210922.7908.0.ced4a272 (#10998)
721575e [JSON-API] Postgres perf job (#10986)
f2d9f07 Release RC2 for SDK 1.17.0 (#10996)
```
Changelog:
```
- [JSON API] Under rare conditions, a multi-template query backed by database
  could have an ACS portion that doesn't match its transaction stream, if
  updated concurrently.  This conditions is now checked and accounted for.
  See `issue #10617 <https://github.com/digital-asset/daml/pull/10617>`__.
- The OAuth2 Middleware now obfuscates its Client Secret when logging
  its config.
- [Integration Kit] We have added ``loggingContext`` as an implicit
  parameter to more _kvutils_ trait methods. Implementors may need to do
  the same in their trait implementations. This can make it easier to
  log with the appropriate context.
java-client-bindings - the original full completion is included in the `CompletionResponse` when available
Daml on SQL, Integration Kit, Sandbox: Drop support for TLS 1.0 and 1.1 in Ledger API.
```

CHANGELOG_BEGIN
CHANGELOG_END
stefanobaghino-da pushed a commit that referenced this pull request Sep 29, 2021
@azure-pipelines
release 1.18.0-snapshot.20210928.7948.0.b4d00317 (#11058)
f13c6d6 
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.

@stefanobaghino-da is in charge of this release.

Commit log:
```
b4d0031 detect unsynchronized contract table and retry (#10617)
3d779cf [Mutable state cache] Fix initialization offset (#11024)
5458aa8 Switch on NonUnitStatements warning in daml-lf/transaction (#11048)
9641fd5 auth middleware: no print secret (#11050)
a885f52 [DPP-417] [DPP-595] Add error code version switching mechanism (#11035)
12e0c72 fix blackduck logic (#11049)
eb87b34 kvutils: Add the logging context for ledger state operations. (#11030)
b7daa5f Address remaining dependabot alerts (#11045)
5e43f8c es: drop jobs-* indices (#10857)
03203b7 Define encoding/decoding for module imports (#11036)
57a1597 Setting timeoutToleranceMillis to 10 minutes to prevent flakiness (#11043)
df59f3f Fix Navigator dependabot alerts (#11044)
6bf45a3 Upgrade Navigator to Webpack 5 (#11040)
80e217e [DPP-622] Add conformance tests that verifies TLSv1.0 and TLSv1 are disabled. (#10983)
626e1fb Small lf value.cids regression fix (found by canton unit tests) (#11032)
a4629a4 KV: Ignore daml_lf_1.proto when checking for KV protobuf compatibility (#11021)
ee9be65 kvutils: Add metadata to `Err` [KVL-1032] (#10992)
0d3ae6e interface methods: Haskell Typechecker (#11028)
e36eb46 Resolve `set-value` to 4.0.1 and above (#11029)
4075624 interface methods: Haskell AST (#11018)
7c1fd50 Bump grunt-browserify (#11026)
5f3f582 Upgrade webpack-dev-server in Navigator (#11025)
91be1e1 Drop matchdep dependency from docs build (#11023)
fe9aeff Increase es disk size (#11019)
eac7963 LF: Refactor ProtoTest.scala (#11020)
e79a30a For the client binding propagate the full original completion [KVL-1112] (#10879)
59ad995 fix buf check (#11014)
abc3e66 Increase the tolarance for handover of control in HaCoordinatorSpec (#10997)
19b2bf4 LF: Cosmetic clean-up in the Speedy Compiler (#11015)
cb0e41f LF: Add interface support to the Preprocessor (#11013)
c33297c Remove `transactionNormalization` flag. (#11010)
f5d2135 Check protobuf compatibility of `main` and PR commits w.r.t. previous stable release [KVL-1109] (#10950)
bf8b75d interface methods: Add protobuf definitions. (#11005)
88e1430 Make LargeTransactionTest use ValueEnricher, so it can work with normalized transactions coming out of the engine. (#11003)
a043926 rotate release duty after 1.17.0-snapshot.20210921.7889.0.1b473c2b (#10972)
35666ca Add MINIMAL pragma for Additive type class (#11001)
8de162b [DPP-586] Upgrade to netty 4.1.67.Final and netty-tcnative-boringssl-static 2.0.40.Final (#10956)
871d03b release 1.18.0-snapshot.20210922.7908.0.ced4a272 (#10998)
721575e [JSON-API] Postgres perf job (#10986)
f2d9f07 Release RC2 for SDK 1.17.0 (#10996)
```
Changelog:
```
- [JSON API] Under rare conditions, a multi-template query backed by database
  could have an ACS portion that doesn't match its transaction stream, if
  updated concurrently.  This conditions is now checked and accounted for.
  See `issue #10617 <https://github.com/digital-asset/daml/pull/10617>`__.
- The OAuth2 Middleware now obfuscates its Client Secret when logging
  its config.
- [Integration Kit] We have added ``loggingContext`` as an implicit
  parameter to more _kvutils_ trait methods. Implementors may need to do
  the same in their trait implementations. This can make it easier to
  log with the appropriate context.
java-client-bindings - the original full completion is included in the `CompletionResponse` when available
Daml on SQL, Integration Kit, Sandbox: Drop support for TLS 1.0 and 1.1 in Ledger API.
```

CHANGELOG_BEGIN
CHANGELOG_END

Co-authored-by: Azure Pipelines DAML Build <support@digitalasset.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabiotudone-da fabiotudone-da fabiotudone-da left review comments

@miklos-da miklos-da miklos-da left review comments

@meiersi-da meiersi-da Awaiting requested review from meiersi-da

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hubert-da @miklos-da @fabiotudone-da