You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by zentol <gi...@git.apache.org> on 2018/01/29 10:55:17 UTC
[GitHub] flink pull request #5378: [FLINK-8489][ES] Prevent side-effects when modifyi...
GitHub user zentol opened a pull request:
https://github.com/apache/flink/pull/5378
[FLINK-8489][ES] Prevent side-effects when modifying user-config
## What is the purpose of the change
The `ElasticsearchSinkBase` constructor was removing entries from a user-provided collection. We now copy the collection to prevent side-effects.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/zentol/flink 8489
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/5378.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #5378
----
commit 3b81b7d3b0f3ae3405f7bc4e5f438af87aa45630
Author: zentol <ch...@...>
Date: 2018-01-29T10:46:08Z
[FLINK-8489][ES] Prevent side-effects when modifying user-config
----
---
[GitHub] flink issue #5378: [FLINK-8489][ES] Prevent side-effects when modifying user...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/5378
merging,
---
[GitHub] flink pull request #5378: [FLINK-8489][ES] Prevent side-effects when modifyi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/flink/pull/5378
---
[GitHub] flink issue #5378: [FLINK-8489][ES] Prevent side-effects when modifying user...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/5378
I'll add a test where we pass an unmodifiable map to the sink, that should cover it.
---
[GitHub] flink issue #5378: [FLINK-8489][ES] Prevent side-effects when modifying user...
Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:
https://github.com/apache/flink/pull/5378
Looks good, but can this be guarded by a test?
Otherwise I can see a someone in the future going like "oh, I have an idea how to 'optimize' this" and remove that copying operation.
---
[GitHub] flink issue #5378: [FLINK-8489][ES] Prevent side-effects when modifying user...
Posted by zentol <gi...@git.apache.org>.
Github user zentol commented on the issue:
https://github.com/apache/flink/pull/5378
Added a test for the `ElasticSearchSinkBase` that passes an unmodifiable map containing all properties that we currently try to remove.
I also modified the `ElasticSearchTestBase` to pass an unmodifiable map to the constructors.
---
[GitHub] flink issue #5378: [FLINK-8489][ES] Prevent side-effects when modifying user...
Posted by tzulitai <gi...@git.apache.org>.
Github user tzulitai commented on the issue:
https://github.com/apache/flink/pull/5378
LGTM, once Travis is green.
---