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

Resolves #1692: Remove ERROR level log on failed no-ops #1693

Merged
merged 1 commit into from May 24, 2022

Conversation

alecgrieser
Copy link
Contributor

This removes the ERROR-level log on no-op failure. The error was already getting propagated, and that, in practice, should be enough for adopters to detect errors and (if they so desire) log the error. I considered adding logic to include the cluster as logging details, but that ended up being a little bit delicate because it only works if the underlying failure is a LoggableException, which isn't necessarily the case because we haven't called the error-wrapping logic at that point. So, it was easier to just leave it without that information, and it didn't seem like a huge loss, as the caller should be able to determine that based on the fact that they need an FDBDatabase to call this on anyway.

This resolves #1692.

This removes the `ERROR`-level log on no-op failure. The error was already getting propagated, and that, in practice, should be enough for adopters to detect errors and (if they so desire) log the error. I considered adding logic to include the `cluster` as logging details, but that ended up being a little bit delicate because it only works if the underlying failure is a `LoggableException`, which isn't necessarily the case because we haven't called the error-wrapping logic at that point. So, it was easier to just leave it without that information, and it didn't seem like a huge loss, as the caller should be able to determine that based on the fact that they need an `FDBDatabase` to call this on anyway.

This resolves FoundationDB#1692.
@alecgrieser alecgrieser marked this pull request as ready for review May 24, 2022 21:49
@sonarcloud
Copy link

sonarcloud bot commented May 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell B 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@foundationdb-ci
Copy link
Contributor

AWS CodeBuild CI Report for Linux CentOS 7

  • CodeBuild project: fdb-record-layer-pr-proto2
  • Commit ID: 7463ac8
  • Result: SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@foundationdb-ci
Copy link
Contributor

AWS CodeBuild CI Report for Linux CentOS 7

  • CodeBuild project: fdb-record-layer-pr-proto3
  • Commit ID: 7463ac8
  • Result: SUCCEEDED
  • Error: N/A
  • Build Logs (available for 30 days)

@alecgrieser alecgrieser requested a review from MMcM May 24, 2022 22:11
@MMcM MMcM merged commit d0e98a7 into FoundationDB:main May 24, 2022
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.

Remove ERROR level log on failed no-ops
3 participants