Skip to content

Conversation

@RobertIndie
Copy link
Member

@RobertIndie RobertIndie commented Sep 19, 2023

Motivation

We have introduced the SinkContext.fatal method in #21143
This PR use this fatal method to handle the elastic sink connector exception correctly.

Modifications

  • Use SinkContext.fatal to throw connector exception
  • Add state to the ElasticSearchClient

Verifying this change

  • Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@RobertIndie RobertIndie self-assigned this Sep 19, 2023
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 19, 2023
@RobertIndie RobertIndie marked this pull request as ready for review September 20, 2023 10:03
@RobertIndie RobertIndie marked this pull request as draft September 20, 2023 12:41
@RobertIndie RobertIndie marked this pull request as ready for review September 21, 2023 10:40
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

@nicoloboschi PTAL

@codecov-commenter
Copy link

Codecov Report

Merging #21204 (45ed5f3) into master (b10eed6) will increase coverage by 36.41%.
Report is 17 commits behind head on master.
The diff coverage is 71.42%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #21204       +/-   ##
=============================================
+ Coverage     36.80%   73.21%   +36.41%     
- Complexity    12195    32469    +20274     
=============================================
  Files          1698     1887      +189     
  Lines        130396   140157     +9761     
  Branches      14253    15428     +1175     
=============================================
+ Hits          47988   102612    +54624     
+ Misses        76079    29454    -46625     
- Partials       6329     8091     +1762     
Flag Coverage Δ
inttests 24.15% <ø> (+0.07%) ⬆️
systests 24.81% <0.00%> (-0.22%) ⬇️
unittests 72.49% <71.42%> (+40.49%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...che/pulsar/io/elasticsearch/ElasticSearchSink.java 75.75% <55.55%> (+75.75%) ⬆️
...e/pulsar/io/elasticsearch/ElasticSearchClient.java 78.82% <100.00%> (+78.82%) ⬆️

... and 1451 files with indirect coverage changes

@RobertIndie RobertIndie merged commit dcf1ea1 into apache:master Sep 25, 2023
liangyuanpeng pushed a commit to liangyuanpeng/pulsar that referenced this pull request Oct 11, 2023
…ache#21204)

### Motivation

We have introduced the `SinkContext.fatal` method in apache#21143
This PR use this fatal method to handle the elastic sink connector exception correctly.

### Modifications

- Use `SinkContext.fatal` to throw connector exception
- Add state to the ElasticSearchClient
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-not-needed Your PR changes do not impact docs ready-to-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants