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

Support rollback nodes in PostCommitValidation #9501

Merged
merged 7 commits into from
Apr 27, 2021
Merged

Support rollback nodes in PostCommitValidation #9501

merged 7 commits into from
Apr 27, 2021

Conversation

cocreature
Copy link
Contributor

With that and the other in-flight PRs, #9400 now passes against Daml
on SQL.

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

With that and the other in-flight PRs, #9400 now passes against Daml
on SQL.

changelog_begin
changelog_end
case ((created, ids), _) => (created, ids)
}
referred.diff(createdInTransaction)
transaction.inputContracts.diff(divulged)
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is a one-liner used only in one place I'd suggest to just inline it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered it but it seemed easier for review at least to only replace the implementation but leave it in a separate definition. Don’t feel strongly though so happy to inline it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call as a first step. 🙇🏻

case ((created, ids), _) => (created, ids)
}
referred.diff(createdInTransaction)
transaction.inputContracts.diff(divulged)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, I'd recommend to just inline this.

cocreature and others added 4 commits April 27, 2021 09:56
…re/appendonlydao/events/PostCommitValidation.scala

Co-authored-by: Stefano Baghino <[email protected]>
…re/appendonlydao/events/PostCommitValidation.scala

Co-authored-by: Stefano Baghino <[email protected]>
…re/dao/events/PostCommitValidation.scala

Co-authored-by: Stefano Baghino <[email protected]>
…re/dao/events/PostCommitValidation.scala

Co-authored-by: Stefano Baghino <[email protected]>
Copy link
Contributor

@nickchapman-da nickchapman-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

cocreature and others added 2 commits April 27, 2021 16:03
changelog_begin
changelog_end
@mergify mergify bot merged commit c567680 into main Apr 27, 2021
@mergify mergify bot deleted the rollback-pcv branch April 27, 2021 17:22
azure-pipelines bot pushed a commit that referenced this pull request Apr 28, 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.

@nickchapman-da is in charge of this release.

Commit log:
```
e1e878a Simplify opt-in/out of Oracle when building (#9515)
d761853 KVL-874 Add unit tests for Telemetry (#9500)
c567680 Support rollback nodes in PostCommitValidation (#9501)
a335ee8 Support rollback nodes in KeyValueCommitting.submissionOutputs (#9512)
8747b3d Support rollback nodes in TransactionIndexing (#9506)
c4cf3c9 kvutils: Log a missing input state warning without the stack trace. (#9513)
4e712a0 add oracle option for json-api perf runner (#9492)
8cd3658 Switch to an environment variable for enabling Oracle tests. (#9511)
e39c20e update GPG public key (#9488)
782109d update LATEST (#9508)
3e66611 Nest stakeholders in contracts table as JSON arrays (#9484)
aecdc2a update NOTICES file (#9507)
```
Changelog:
```
- [JSON-API Perf] ``--query-store-index=postgres`` must be passed
  to select PostgreSQL query store performance testing; ``true``
  and ``yes`` are no longer supported.
  See `issue #9492 <https://github.com/digital-asset/daml/pull/9492>`__.
```

CHANGELOG_BEGIN
CHANGELOG_END
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