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

Clean up kvutils logs #3780

Merged
merged 4 commits into from
Dec 10, 2019
Merged

Clean up kvutils logs #3780

merged 4 commits into from
Dec 10, 2019

Conversation

mziolekda
Copy link
Contributor

  • Clean up kvutils logs
  • Convert config validation to be Committer-based

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, if appropriate
  • 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.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

few comments, otherwise lgtm.

logger.trace(
s"Configuration rejected, $msg, correlationId=${configurationEntry.getSubmissionId}")

private val checkTtl: Step = (ctx, result) => {
Copy link

Choose a reason for hiding this comment

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

consider already in this PR pulling the checkTtl and authorizeSubmission to be built into Committer (we'll of course need to specify how to build the timeout/participant not authorized results).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haven't done that one. The commonalities are still too hard to spot through the thicket of diverging abstractions.

@mziolekda mziolekda force-pushed the clean-up-kvutils-logs branch from b89b0cf to 485c0c8 Compare December 10, 2019 08:35
@mziolekda mziolekda merged commit 3c48bd3 into master Dec 10, 2019
@mziolekda mziolekda deleted the clean-up-kvutils-logs branch December 10, 2019 10:12
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.

1 participant