ledger-configuration: Extract the configuration from participant-state. [KVL-1002] #10279

ghost · 2021-07-14T13:42:22Z

The configuration is often used without the state, and it probably doesn't need to be versioned in the same way.

This is a big PR. Apologies for that. It's pretty much entirely renames and repackaging; no behavior has changed except that we now publish an extra Maven package, ledger-configuration.

Changelog

[Integration Kit] The ledger configuration classes, Configuration, LedgerInitialConditions, and TimeModel, have been moved from participant-state to a separate package named ledger-configuration, in the Java package com.daml.ledger.configuration. You will need to update your dependencies and imports.
[Integration Kit] TimeModel has been renamed to LedgerTimeModel. If you are using the ledger configuration classes directly, you may need to update your code.

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.

The configuration is often used without the state, and doesn't need to be versioned in the same way. CHANGELOG_BEGIN - [Integration Kit] The ledger configuration classes, ``Configuration``, ``LedgerInitialConditions``, and ``TimeModel``, have been moved from *participant-state* to a separate package named *ledger-configuration*, in the Java package ``com.daml.ledger.configuration``. You will need to update your dependencies and imports. CHANGELOG_END

This avoids confusion with the protobuf-generated `TimeModel` classes. CHANGELOG_BEGIN - [Integration Kit] ``TimeModel`` has been renamed to ``LedgerTimeModel``. If you are using the ledger configuration classes directly, you may need to update your code. CHANGELOG_END

ghost · 2021-07-14T13:42:50Z

@gerolf-da, would you mind chiming in on whether this is a good idea or not?

ghost · 2021-07-14T14:27:29Z

To add a bit more context: the purpose of this is to decouple the participant-state-index package from participant-state, so we can use v1 and v2 of the participant state interchangeably.

If this is acceptable, I will follow up with PRs to move Offset into a new package, and use the Daml-LF Ref object directly instead of the type aliases in v1/package.scala.

…ation

hubert-da

LGTM at a low level. You may want to wait for a higher level approval ;)

hubert-da · 2021-07-14T14:53:32Z

...icipant-state/src/main/scala/com/daml/ledger/participant/state/v2/AdaptedV1ReadService.scala

@@ -33,29 +32,6 @@ class AdaptedV1ReadService(delegate: v1.ReadService) extends ReadService {
 }

 private[v2] object AdaptedV1ReadService {
-  def adaptLedgerInitialConditions(


hubert-da · 2021-07-14T14:53:50Z

...cipant-state/src/main/scala/com/daml/ledger/participant/state/v2/AdaptedV1WriteService.scala

@@ -163,18 +164,4 @@ private[v2] object AdaptedV1WriteService {
    val errorInfo = ErrorInfo.of(failure.getLocalizedMessage, "Synchronous rejection", metadata)
    Seq(com.google.protobuf.any.Any.pack(errorInfo))
  }
-
-  def adaptLedgerConfiguration(config: Configuration): v1.Configuration =


gerolf-da

@SamirTalwar-DA: I'd like to understand the motivation for this change a bit better, please.

ledger/ledger-configuration/protobuf/ledger_configuration.rst

...er/participant-state/src/main/scala/com/daml/ledger/participant/state/v2/Configuration.scala

ledger/sandbox-common/src/main/scala/platform/sandbox/cli/CommonCliBase.scala

ghost · 2021-07-15T07:34:31Z

@gerolf-da: Happy to discuss any concerns here or over a call. 😃

Discussed offline.

Co-authored-by: Miklos <[email protected]>

fabiotudone-da

I am not so confident about the ledger time model not having the same versioning model of the API, as it is (also) part of it; anyway, supposing we merge this change and then we need to start versioning it, how would you do it?

ledger/ledger-configuration/protobuf/ledger_configuration.rst

ghost · 2021-07-15T08:50:36Z

@fabiotudone-da: The protobuf, ledger_configuration.proto, is versioned, which I think takes care of that problem. The classes can (I believe) be evolved safely, as long as they're always able to decode later versions of the protobuf.

Take a look at Configuration.scala. It has two decode methods, where v1 provides a sensible default for maxDeduplicationTime.

Do you think this satisfies your concern?

rautenrieth-da

I had a quick high-level look at these changes.

My only concern was that now two different versions of the API (v1 and v2) depend on the same types, which means we'll have to be careful when changing those types (e.g., if TimeModel should change in v2 but not in v1). After discussion with @SamirTalwar-DA, I do not think this is an issue because:

We do not expect changes to the time model and the configuration
The v1 and v2 versions behave more like the stable and preview version of "the" participant state API, the main versioning of which is the SDK version.

fabiotudone-da · 2021-07-15T08:57:11Z

I had a quick high-level look at these changes.

My only concern was that now two different versions of the API (v1 and v2) depend on the same types, which means we'll have to be careful when changing those types (e.g., if TimeModel should change in v2 but not in v1). After discussion with @SamirTalwar-DA, I do not think this is an issue because:

We do not expect changes to the time model and the configuration

The v1 and v2 versions behave more like the stable and preview version of "the" participant state API, the main versioning of which is the SDK version.

@SamirTalwar-DA @rautenrieth-da I think this satisfies my concerns. Thanks!

ghost · 2021-07-15T08:58:31Z

OK, I'm going to merge this, with a bit of trepidation.

Worst-case scenario, we revert.

…ation

@Robin-da

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: ``` b7cf42d Upgrade doobie to version 0.13.4 (#10326) 1a72314 fix broken triggers link (#10336) 271f9da Added back DeduplicationPeriod.DeduplicationOffset [KVL-998] (#10324) b701caa Report divulgence warning at commit location. (#10329) 742bdcb Describe prometheus in daml-on-sql README (#10307) 93d0ed6 [Mutable cache] Resolve with full lookup on negative cache read-through lookups [DPP-501] (#10262) f59951b document min npm version (#10330) d982e16 fix besu listing (#10328) c3bc26f daml-lf/data: Move ID aliases to `Ref` from _ledger-api-common_. [KVL-1002] (#10323) cf2f79d Register divulgence warning as a diagnostic (#10289) 8360e9f Mark Java Bot tests flaky (#10322) a6bf892 [In-memory fan-out] CLI parameters update (#10250) 5fa49c3 Disables flaky test suite of InstrumentedSource.bufferedSource (#10321) 504c231 Use a combined template id, stakeholder index (#10315) 9a01065 participant-state: Remove the Daml-LF transaction and value aliases. [KVL-1002] (#10317) 159728d LF: use Either by default in all archive reader API (#10295) 449a72a increase ES memory (#10318) 80b65f6 [DOCS] Add documentation for the JSON API metrics (#10312) 6abb903 [JSON-API] Put the request & response bodies into the log ctx if the log level is debug (for specific statements) (#10298) 3c0010b detect mount issue earlier (#10313) 0e09f3b Remove optLocation field from transaction nodes. (#10301) e62852f Make ledger API test tool's 'max-connection-attempts' into an option (#10311) bd12170 participant-state: Remove the `ParticipantId`, `PackageId`, and `Party` aliases. [KVL-1002] (#10308) fed4497 Extend JSON API ContractDao query bench’s with different tpids (#10309) bd35f80 ledger-configuration: Correct the protobuf package. [KVL-1002] (#10302) d69328d Ledger API Test Tool: add a `--max-connection-attempts` command line option [KVL-977] (#10297) a3b861e refresh es cluster (#10300) 63332d0 update NOTICES file (#10306) 1ee53c0 participant-state-index: Use `Ref` directly. (#10299) 0eba812 Remove trace_context field from Ledger API [KVL-1021] (#10256) 88886be ledger-configuration: Add tests for the `Configuration` decoding methods. [KVL-1002] (#10293) cb29f34 ledger-offset: Move `Offset` to a new package. [KVL-1002] (#10296) b899ae1 Codeowners: Add @nicu-da to the kvutils code review. (#10292) f033bc2 LF: Clean legacy from archive proto + TEXT_TO_CODE_POINTS typo (#10286) 70b12f0 ledger-on-sql: Remove conformance tests for H2 and SQLite files. (#10291) a9a0b70 ledger-configuration: Extract the configuration from participant-state. [KVL-1002] (#10279) d96b54e Use Text not String in exception docs (#10287) 5382253 Refresh json API materialized view on statement (#10285) 6e447c5 Patch export filtering of GHC.Types.[] in damldocs (#10282) d565cae DPP-457 Add UpdateToDbdto tests (#10116) 814442e update NOTICES file (#10284) 0493480 rotate release duty after 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10268) 84094e0 update compat versions for 1.15.0 (#10281) ba8e829 [In-memory fan-out] Oracle support (#10263) a8f1902 LF: change type from Try to Either in archive module (#10277) c84b37a Release SDK 1.15 (#10269) 980d96b Fix status check in collect_build_data (#10278) 191d3b8 Add unit test to SequentialWriteDao [DPP-456] (#10265) 2271f93 Fix crash on duplicate submissionId (#10242) 906184c LF: Simplify UniversalDarReader (#10271) 42c487b [JSON-API] Refactor Endpoints.scala to use path directives etc. (#10274) 6840401 release 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10267) 0df70fa Document bazel-testlogs directory in BAZEL.md (#10276) 44204bb Add a benchmark for contract insertion in the JSON API (#10272) 6107f8a Ignore failure to upload log failures (#10270) caf85a2 LF: rationalize archive Parser/Reader/Decoder (#10239) 0043b81 Add a ContractDao benchmark (#10259) 21a9e62 Add disclaimer to participant state v2 API [KVL-998] (#10254) 6c63f96 participant-integration-api: Move `SeedService` here. (#10261) 58c0b46 kvutils: Simplify usage of Rejections [KVL-1015] (#10258) 41aba1c kvutils: Move unrelated test cases to a proper test class [KVL-1015] (#10257) ``` Changelog: ``` - [JSON-API] You can now find a section `Metrics` in the http-json api documentation explaining how to enable metrics and which are available - [JSON-API] Logging of the request and response bodies are now available for appropriate requests if the chosen log level is equal or lower than DEBUG. These can then be found in the logging context of the request begin & end log messages (The field names in the ctx are "request_body" and "response_body"). - [Integration Kit] The *ledger_configuration.proto* Protobuf definition has been repackaged under ``com.daml.ledger.configuration``, and the Java and C# packages have been renamed accordingly. If you are using this Protobuf definition, you will need to update your imports. The Maven artifact name has not changed. - [Integration Kit] Add a `--max-connection-attempts` command line option to the Ledger API Test Tool * [Integration Kit] Removed trace_context field from Ledger API and its bindings as we now have trace context propagation support via gRPC metadata. If you are constructing or consuming Ledger API requests or responses directly, you may need to update your code. - [Integration Kit] The ``Offset`` type has been moved to a new Maven package, ``ledger-offset``, from the ``participant-state`` package. The Java package has been renamed to ``com.daml.ledger.offset``. If you are using this type, you will need to update your dependencies and imports. - [Integration Kit] The ledger configuration classes, ``Configuration``, ``LedgerInitialConditions``, and ``TimeModel``, have been moved from *participant-state* to a separate package named *ledger-configuration*, in the Java package ``com.daml.ledger.configuration``. You will need to update your dependencies and imports. - [Integration Kit] ``TimeModel`` has been renamed to ``LedgerTimeModel``. If you are using the ledger configuration classes directly, you may need to update your code. Fix crash in ConfigManagement- and PackageManagement- services on duplicate submissionsIds from different participants. Extend ledger-api-test-tool to cover the duplicate submissionId cases. - [Integration Kit] The class ``SeedService`` has been moved from the *participant-state* Maven package to the *participant-integration-api* Maven package, under the Java package name ``com.daml.platform.apiserver`` to reflect its usage by the API server, not the participant state API. If you use this class directly, you will need to change your imports. ``` CHANGELOG_BEGIN CHANGELOG_END

@Robin-da

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: ``` b7cf42d Upgrade doobie to version 0.13.4 (#10326) 1a72314 fix broken triggers link (#10336) 271f9da Added back DeduplicationPeriod.DeduplicationOffset [KVL-998] (#10324) b701caa Report divulgence warning at commit location. (#10329) 742bdcb Describe prometheus in daml-on-sql README (#10307) 93d0ed6 [Mutable cache] Resolve with full lookup on negative cache read-through lookups [DPP-501] (#10262) f59951b document min npm version (#10330) d982e16 fix besu listing (#10328) c3bc26f daml-lf/data: Move ID aliases to `Ref` from _ledger-api-common_. [KVL-1002] (#10323) cf2f79d Register divulgence warning as a diagnostic (#10289) 8360e9f Mark Java Bot tests flaky (#10322) a6bf892 [In-memory fan-out] CLI parameters update (#10250) 5fa49c3 Disables flaky test suite of InstrumentedSource.bufferedSource (#10321) 504c231 Use a combined template id, stakeholder index (#10315) 9a01065 participant-state: Remove the Daml-LF transaction and value aliases. [KVL-1002] (#10317) 159728d LF: use Either by default in all archive reader API (#10295) 449a72a increase ES memory (#10318) 80b65f6 [DOCS] Add documentation for the JSON API metrics (#10312) 6abb903 [JSON-API] Put the request & response bodies into the log ctx if the log level is debug (for specific statements) (#10298) 3c0010b detect mount issue earlier (#10313) 0e09f3b Remove optLocation field from transaction nodes. (#10301) e62852f Make ledger API test tool's 'max-connection-attempts' into an option (#10311) bd12170 participant-state: Remove the `ParticipantId`, `PackageId`, and `Party` aliases. [KVL-1002] (#10308) fed4497 Extend JSON API ContractDao query bench’s with different tpids (#10309) bd35f80 ledger-configuration: Correct the protobuf package. [KVL-1002] (#10302) d69328d Ledger API Test Tool: add a `--max-connection-attempts` command line option [KVL-977] (#10297) a3b861e refresh es cluster (#10300) 63332d0 update NOTICES file (#10306) 1ee53c0 participant-state-index: Use `Ref` directly. (#10299) 0eba812 Remove trace_context field from Ledger API [KVL-1021] (#10256) 88886be ledger-configuration: Add tests for the `Configuration` decoding methods. [KVL-1002] (#10293) cb29f34 ledger-offset: Move `Offset` to a new package. [KVL-1002] (#10296) b899ae1 Codeowners: Add @nicu-da to the kvutils code review. (#10292) f033bc2 LF: Clean legacy from archive proto + TEXT_TO_CODE_POINTS typo (#10286) 70b12f0 ledger-on-sql: Remove conformance tests for H2 and SQLite files. (#10291) a9a0b70 ledger-configuration: Extract the configuration from participant-state. [KVL-1002] (#10279) d96b54e Use Text not String in exception docs (#10287) 5382253 Refresh json API materialized view on statement (#10285) 6e447c5 Patch export filtering of GHC.Types.[] in damldocs (#10282) d565cae DPP-457 Add UpdateToDbdto tests (#10116) 814442e update NOTICES file (#10284) 0493480 rotate release duty after 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10268) 84094e0 update compat versions for 1.15.0 (#10281) ba8e829 [In-memory fan-out] Oracle support (#10263) a8f1902 LF: change type from Try to Either in archive module (#10277) c84b37a Release SDK 1.15 (#10269) 980d96b Fix status check in collect_build_data (#10278) 191d3b8 Add unit test to SequentialWriteDao [DPP-456] (#10265) 2271f93 Fix crash on duplicate submissionId (#10242) 906184c LF: Simplify UniversalDarReader (#10271) 42c487b [JSON-API] Refactor Endpoints.scala to use path directives etc. (#10274) 6840401 release 1.15.0-snapshot.20210713.7343.0.1f35db17 (#10267) 0df70fa Document bazel-testlogs directory in BAZEL.md (#10276) 44204bb Add a benchmark for contract insertion in the JSON API (#10272) 6107f8a Ignore failure to upload log failures (#10270) caf85a2 LF: rationalize archive Parser/Reader/Decoder (#10239) 0043b81 Add a ContractDao benchmark (#10259) 21a9e62 Add disclaimer to participant state v2 API [KVL-998] (#10254) 6c63f96 participant-integration-api: Move `SeedService` here. (#10261) 58c0b46 kvutils: Simplify usage of Rejections [KVL-1015] (#10258) 41aba1c kvutils: Move unrelated test cases to a proper test class [KVL-1015] (#10257) ``` Changelog: ``` - [JSON-API] You can now find a section `Metrics` in the http-json api documentation explaining how to enable metrics and which are available - [JSON-API] Logging of the request and response bodies are now available for appropriate requests if the chosen log level is equal or lower than DEBUG. These can then be found in the logging context of the request begin & end log messages (The field names in the ctx are "request_body" and "response_body"). - [Integration Kit] The *ledger_configuration.proto* Protobuf definition has been repackaged under ``com.daml.ledger.configuration``, and the Java and C# packages have been renamed accordingly. If you are using this Protobuf definition, you will need to update your imports. The Maven artifact name has not changed. - [Integration Kit] Add a `--max-connection-attempts` command line option to the Ledger API Test Tool * [Integration Kit] Removed trace_context field from Ledger API and its bindings as we now have trace context propagation support via gRPC metadata. If you are constructing or consuming Ledger API requests or responses directly, you may need to update your code. - [Integration Kit] The ``Offset`` type has been moved to a new Maven package, ``ledger-offset``, from the ``participant-state`` package. The Java package has been renamed to ``com.daml.ledger.offset``. If you are using this type, you will need to update your dependencies and imports. - [Integration Kit] The ledger configuration classes, ``Configuration``, ``LedgerInitialConditions``, and ``TimeModel``, have been moved from *participant-state* to a separate package named *ledger-configuration*, in the Java package ``com.daml.ledger.configuration``. You will need to update your dependencies and imports. - [Integration Kit] ``TimeModel`` has been renamed to ``LedgerTimeModel``. If you are using the ledger configuration classes directly, you may need to update your code. Fix crash in ConfigManagement- and PackageManagement- services on duplicate submissionsIds from different participants. Extend ledger-api-test-tool to cover the duplicate submissionId cases. - [Integration Kit] The class ``SeedService`` has been moved from the *participant-state* Maven package to the *participant-integration-api* Maven package, under the Java package name ``com.daml.platform.apiserver`` to reflect its usage by the API server, not the participant state API. If you use this class directly, you will need to change your imports. ``` CHANGELOG_BEGIN CHANGELOG_END Co-authored-by: Azure Pipelines DAML Build <[email protected]>

SamirTalwar added 4 commits July 14, 2021 15:25

participant-state: Remove the LedgerId aliases.

32cabcd

ledger-configuration: Remove colons in LedgerInitialConditions' docs.

4e7f7cf

ghost requested review from fabiotudone-da, gerolf-da, miklos-da, mziolekda, nmarton-da, rautenrieth-da and tudor-da as code owners July 14, 2021 13:42

ghost requested review from hubert-da and dasormeter July 14, 2021 13:42

SamirTalwar added 2 commits July 14, 2021 16:07

kvutils: Restore a missing compat import.

3e3b09b

participant-integration-api: Add ledger-configuration to Oracle tests.

6b93102

Merge remote-tracking branch 'origin/main' into samir/ledger-configur…

439766e

…ation

hubert-da approved these changes Jul 14, 2021

View reviewed changes

gerolf-da previously requested changes Jul 14, 2021

View reviewed changes

miklos-da approved these changes Jul 14, 2021

View reviewed changes

sandbox-common: Fix --max-ledger-time-skew docs.

ff76a67

Co-authored-by: Miklos <[email protected]>

fabiotudone-da reviewed Jul 15, 2021

View reviewed changes

ledger/ledger-configuration/protobuf/ledger_configuration.rst Show resolved Hide resolved

rautenrieth-da reviewed Jul 15, 2021

View reviewed changes

fabiotudone-da approved these changes Jul 15, 2021

View reviewed changes

ghost added the automerge label Jul 15, 2021

Merge remote-tracking branch 'origin/main' into samir/ledger-configur…

48b65af

…ation

mergify bot merged commit a9a0b70 into main Jul 15, 2021

mergify bot deleted the samir/ledger-configuration branch July 15, 2021 13:03

This was referenced Jul 15, 2021

ledger-offset: Move Offset to a new package. [KVL-1002] #10296

Merged

ledger-configuration: Correct the protobuf package. [KVL-1002] #10302

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ledger-configuration: Extract the configuration from participant-state. [KVL-1002] #10279

ledger-configuration: Extract the configuration from participant-state. [KVL-1002] #10279

ghost commented Jul 14, 2021

ghost commented Jul 14, 2021

ghost commented Jul 14, 2021

hubert-da left a comment

hubert-da Jul 14, 2021

hubert-da Jul 14, 2021

gerolf-da left a comment

ghost commented Jul 15, 2021

fabiotudone-da left a comment

ghost commented Jul 15, 2021

rautenrieth-da left a comment

fabiotudone-da commented Jul 15, 2021

ghost commented Jul 15, 2021

ledger-configuration: Extract the configuration from participant-state. [KVL-1002] #10279

ledger-configuration: Extract the configuration from participant-state. [KVL-1002] #10279

Conversation

ghost commented Jul 14, 2021

Changelog

Pull Request Checklist

ghost commented Jul 14, 2021

ghost commented Jul 14, 2021

hubert-da left a comment

Choose a reason for hiding this comment

hubert-da Jul 14, 2021

Choose a reason for hiding this comment

hubert-da Jul 14, 2021

Choose a reason for hiding this comment

gerolf-da left a comment

Choose a reason for hiding this comment

ghost commented Jul 15, 2021

fabiotudone-da left a comment

Choose a reason for hiding this comment

ghost commented Jul 15, 2021

rautenrieth-da left a comment

Choose a reason for hiding this comment

fabiotudone-da commented Jul 15, 2021

ghost commented Jul 15, 2021