Skip to content

fix: publishing runtime error as a WriteErrorEvent #292

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

Merged
merged 2 commits into from
Jan 19, 2022
Merged

fix: publishing runtime error as a WriteErrorEvent #292

merged 2 commits into from
Jan 19, 2022

Conversation

jsimomaa
Copy link
Contributor

@jsimomaa jsimomaa commented Dec 16, 2021

Closes #291

Proposed Changes

Publishing runtime error as a WriteErrorEvent

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • mvn test completes successfully
  • Commit messages are in semantic format
  • Sign CLA (if not already signed)

@bednar bednar changed the title Throwable not published as WriteErrorEvent #291 fix: publishing runtime error as a WriteErrorEvent Jan 11, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2022

Codecov Report

Merging #292 (cbfc72c) into master (c592324) will not change coverage.
The diff coverage is 100.00%.

❗ Current head cbfc72c differs from pull request most recent head 4058779. Consider uploading reports for the commit 4058779 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##             master     #292   +/-   ##
=========================================
  Coverage     88.92%   88.92%           
  Complexity      473      473           
=========================================
  Files           149      149           
  Lines          5988     5988           
  Branches        290      290           
=========================================
  Hits           5325     5325           
  Misses          580      580           
  Partials         83       83           
Impacted Files Coverage Δ
.../influxdb/client/internal/AbstractWriteClient.java 92.34% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c592324...4058779. Read the comment docs.

@bednar bednar self-requested a review January 11, 2022 07:34
Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

Thanks for your PR 👍.

There are a few requirements that must be be satisfy before we accept the PR:

  1. Please fill description of your changes and satisfy checklist:
    image
  2. Please add a test that will cover your changes.

Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

Thanks again for your PR 👍. There are a few requirements that must be be satisfy before we accept the PR:

  1. Please rebase your sources with master to fix conflicts in CHANGELOG.md.
  2. Please update license header in PublishRuntimeErrorAsWriteErrorEvent.java by: mvn license:format

Regards.

Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

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

We're almost done. There's one last code style requirement:

* https://github.com/influxdata/influxdb-client-java/issues/291
*/
@RunWith(JUnitPlatform.class)
public class PublishRuntimeErrorAsWriteErrorEvent extends AbstractInfluxDBClientTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rename class and file to PublishRuntimeErrorAsWriteErrorEventTest.

Suggested change
public class PublishRuntimeErrorAsWriteErrorEvent extends AbstractInfluxDBClientTest {
public class PublishRuntimeErrorAsWriteErrorEventTest extends AbstractInfluxDBClientTest {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I pushed this change

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks

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 noticed I forgot to save the refactor after the file rename and you had to rename the class yourself, thank you!

@bednar bednar merged commit 9516629 into influxdata:master Jan 19, 2022
@bednar bednar added this to the 4.1.0 milestone Jan 19, 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.

AbstractWriteClient throwable not published as WriteErrorEvent
3 participants