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

participant-integration-api: Switch to the v2 participant state API. [KVL-1002] #10398

Merged
merged 11 commits into from
Jul 28, 2021

Conversation

ghost
Copy link

@ghost ghost commented Jul 23, 2021

In which we draw the rest of the owl.

Switching to the v2 participant state API means:

  • the API server and indexer expect v2 API traits
    • adapters are provided if you need to elevate your v1 API usage
  • the indexer internally uses v2 Updates
  • rejections are switched over to use the v2 format
  • Sandbox Classic uses v2 as the internal representation too (because it interacts directly with the underlying indexer representation, and is therefore tightly coupled)

kvutils and other users of the StandaloneApiServer and StandaloneIndexerServer use the adapters.

Changelog

  • [Integration Kit] The API server and indexer have switched over to v2 of the participant-state API. You can continue to use the v1 API, but you will need to wrap your ReadService and WriteService objects in the AdaptedV1ReadService and AdaptedV1WriteSerivce classes.

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.

`CompletionInfo` is identical to `SubmitterInfo`. Its purpose is to make
the migration to v2 easier. It should not make it into the final
version.
This means that:

  - the API server and indexer expect v2 API traits
    - adapters are provided if you need to elevate your v1 API usage
  - the indexer internally uses v2 Updates
  - rejections are switched over to use the v2 format
  - Sandbox Classic uses v2 as the internal representation too (because
    it interacts directly with the underlying indexer representation,
    and is therefore tightly coupled)

_kvutils_ and other users of the `StandaloneApiServer` and
`StandaloneIndexerServer` use the adapters.

CHANGELOG_BEGIN
- [Integration Kit] The API server and indexer have switched over to v2
  of the participant-state API. You can continue to use the v1 API, but
  you will need to wrap your ``ReadService`` and ``WriteService``
  objects in the ``AdaptedV1ReadService`` and ``AdaptedV1WriteSerivce``
  classes.
CHANGELOG_END
@ghost ghost force-pushed the samir/participant-integration-api/v2-state branch from 6773acf to 2efbc76 Compare July 26, 2021 13:55
@ghost ghost changed the title participant-integration-api: Switch to the v2 participant state API. participant-integration-api: Switch to the v2 participant state API. [KVL-1002] Jul 26, 2021
@ghost ghost marked this pull request as ready for review July 26, 2021 14:04
@ghost ghost requested a review from miklos-da July 26, 2021 14:04
Copy link
Contributor

@fabiotudone-da fabiotudone-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 (but consider that my context is somewhat limited)

And introduce a trait, because, well, this is still the JVM.
@ghost
Copy link
Author

ghost commented Jul 27, 2021

/azp run

@azure-pipelines
Copy link
Contributor

Azure Pipelines successfully started running 1 pipeline(s).

Comment on lines -167 to -168
case state.SubmissionResult.NotSupported =>
logger.info("Setting an initial ledger configuration is not supported")
Copy link
Author

Choose a reason for hiding this comment

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

@mziolekda pointed out that removing this might cause issues with ledgers + drivers such as Canton that don't support submitting configuration. As these drivers have other ways of managing configuration, I'm going to change LedgerConfiguration to allow them to state that we should never submit a new configuration, instead just waiting indefinitely for one to show up. @oliverse-da has agreed this approach will work for Canton.

I'll make this change separately.

Copy link
Contributor

@mziolekda mziolekda 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!
That's a big PR indeed. Mostly boiler plate on the storage side though.

@ghost
Copy link
Author

ghost commented Jul 28, 2021

Agreed, it's huge. If it helps, imagine how big it would have been if we did it in one PR instead of 17.

@ghost ghost added the automerge label Jul 28, 2021
@mergify mergify bot merged commit d666f76 into main Jul 28, 2021
@mergify mergify bot deleted the samir/participant-integration-api/v2-state branch July 28, 2021 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants