Skip to content

fix: publishing runtime error as a WriteErrorEvent#292

Merged
bednar merged 2 commits intoinfluxdata:masterfrom
jsimomaa:master
Jan 19, 2022
Merged

fix: publishing runtime error as a WriteErrorEvent#292
bednar merged 2 commits intoinfluxdata:masterfrom
jsimomaa:master

Conversation

@jsimomaa
Copy link
Copy Markdown
Contributor

@jsimomaa jsimomaa commented Dec 16, 2021 •
edited
Loading

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
Copy Markdown

codecov-commenter commented Jan 11, 2022 •
edited
Loading

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
bednar
bednar requested changes Jan 11, 2022
View reviewed changes
Copy link
Copy Markdown
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.

@bednar bednar mentioned this pull request Jan 11, 2022
Closed
@jsimomaa jsimomaa requested a review from bednar January 18, 2022 11:40
bednar
bednar requested changes Jan 18, 2022
View reviewed changes
Copy link
Copy Markdown
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.

bednar
bednar requested changes Jan 19, 2022
View reviewed changes
Copy link
Copy Markdown
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:

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

@bednar bednar Jan 19, 2022

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
Copy Markdown
Contributor Author

@jsimomaa jsimomaa Jan 19, 2022

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
Copy Markdown
Contributor

@bednar bednar Jan 19, 2022

Choose a reason for hiding this comment

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

Thanks

Copy link
Copy Markdown
Contributor Author

@jsimomaa jsimomaa Jan 19, 2022

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
rhajek pushed a commit that referenced this pull request Jan 20, 2022
@jsimomaa @rhajek
fix: publishing runtime error as a WriteErrorEvent (#292)
5a1b297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bednar bednar bednar approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

4.1.0

Development

Successfully merging this pull request may close these issues.

AbstractWriteClient throwable not published as WriteErrorEvent

3 participants

@jsimomaa @codecov-commenter @bednar