schulzp opened a new pull request, #83:
URL: https://github.com/apache/flink-connector-elasticsearch/pull/83
Extracted `BulkResponseInspector` interface to allow custom handling of
(partially) failed bulk requests. If not overridden, default behaviour remains
unchanged and partial failures
reswqa commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1853168166
@schulzp That PR has been merged. I have pushed an empty commit to your
branch to re-trigger CI.
--
This is an automated message from the Apache Git Service.
To respon
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1853540370
@reswqa, there are two things that need to be fixed:
1. use `MetricsGroupTestUtils#mockWriterMetricGroup()` instead of
`InternalSinkWriterMetricGroup.mock()`, how
reswqa commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1855090968
@schulzp, thank you for the investigation.
1. I think `TestingSinkWriterMetricGroup` might help.
2. Yes, this is a violations indeed. But I'm not sure if this wi
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1855334073
@reswqa, thanks `TestingSinkWriterMetricGroup` works on all tested versions.
I only faced the arch rules issue locally, so far.
--
This is an automated message from
reswqa commented on code in PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#discussion_r1427487497
##
flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/sink/ElasticsearchSinkBuilderBaseTest.java:
##
@@ -99,7 +
reswqa commented on code in PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#discussion_r1427487497
##
flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/sink/ElasticsearchSinkBuilderBaseTest.java:
##
@@ -99,7 +
reswqa commented on code in PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#discussion_r1427487497
##
flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/sink/ElasticsearchSinkBuilderBaseTest.java:
##
@@ -99,7 +
reswqa commented on code in PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#discussion_r1427487497
##
flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/sink/ElasticsearchSinkBuilderBaseTest.java:
##
@@ -99,7 +
reswqa merged PR #83:
URL: https://github.com/apache/flink-connector-elasticsearch/pull/83
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubs
boring-cyborg[bot] commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1857562560
Awesome work, congrats on your first merged pull request!
--
This is an automated message from the Apache Git Service.
To respond to the message, please
reswqa commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1857562853
Thanks @schulzp, nice one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go t
boring-cyborg[bot] commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1835980923
Thanks for opening this pull request! Please check out our contributing
guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)
--
T
afedulov commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1841113426
@schulzp thanks for the contribution. The approach looks solid. I believe
what is missing are some tests that check that the non-default inspector is
actually utilized
reta commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1842971156
@schulzp there was a similar change introduced into
`flink-connector-opensearch`, I believe we could backport it to the
Elasticsearch connector to have a similar model of
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1843046355
@afedulov, thanks! I added a test that makes sure the inspector is passed to
the `ElasticsearchWriter`.
@reswqa, I looked into the pipeline failed pipelines: exce
reta commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1843053300
> @reta, I checked out that the opensearch failure handler. That achieves
the same in regard of error handling. However, it does not allow capturing
additional metrics. At
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1843099110
@reta, sure, so I'll rename my interfaces to match those from opensearch and
add the factory to the opensearch sink builder afterwards. I assume there
shared code to b
reta commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1843109798
> I assume there shared code to be reused and that is more about API
consistency?
There is no shared code (I think you meant that) and indeed, it is more
about API
afedulov commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1843121034
@schulzp thanks!
We try to avoid Mockito usage, unless it is not possible because because of
external dependencies that required concrete classes rather than interf
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1843137406
@afedulov, sure, I'll implement it with plain java.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub an
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1843307919
@afedulov, fixed the test (and extended it)
@reta, I added a `FailureHandler` with a default implementation that
resembles the current fail-on-any-failure behavio
reswqa commented on code in PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#discussion_r1418211399
##
flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/sink/DefaultBulkResponseInspectorTest.java:
##
@@ -0,0 +1
schulzp commented on code in PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#discussion_r1418486556
##
flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/sink/DefaultBulkResponseInspectorTest.java:
##
@@ -0,0 +
schulzp commented on code in PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#discussion_r1418511130
##
flink-connector-elasticsearch-base/src/test/java/org/apache/flink/connector/elasticsearch/sink/DefaultBulkResponseInspectorTest.java:
##
@@ -0,0 +
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1845356753
@reswqa, I overrode the missing methods required by 1.18.0's
`Sink.InitContext`.
--
This is an automated message from the Apache Git Service.
To respond to the messag
schulzp commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1850111290
@reswqa, fixed the cause of the `NotSerializableException` (implicit
reference of anonymous class to `ElasticsearchSinkBuilderBase`). CI should pass
now.
--
This is
reswqa commented on PR #83:
URL:
https://github.com/apache/flink-connector-elasticsearch/pull/83#issuecomment-1851249351
We should waiting for https://github.com/apache/flink/pull/23876 to be
merged(This won't take long as it has already been approved by 3 votes). After
that, CI should be
28 matches
Mail list logo
