Skip to content

Added ability to delete and update records in ElasticSearch sink connector#7329

Closed
alim-zanibekov wants to merge 2 commits intoapache:masterfrom
alim-zanibekov:feature/elastic-search-sink-delete-update
Closed

Added ability to delete and update records in ElasticSearch sink connector#7329
alim-zanibekov wants to merge 2 commits intoapache:masterfrom
alim-zanibekov:feature/elastic-search-sink-delete-update

Conversation

@alim-zanibekov
Copy link

Motivation

I need to use ElasticSearch via Pulsar as a database but not as a dump of logs that’s why I implemented the possibility to update and delete records, as well as to set id when creating a record

Modifications

Record processing

  • added "ACTION" [Record] property which can be "UPSERT" or "DELETE", default value is "UPSERT"
  • added "ID" [Record] property which provides record id (required for "DELETE")

Tests

  • added updateSingleRecordTest
  • added deleteSingleRecordTest
  • renamed singleRecordTest-> insertSingleRecordTest

Verifying this change

This change added tests and can be verified as follows:

  • Extended unit tests (ElasticSearchSinkTests) for update and delete requests

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

  • Dependencies (does it add or upgrade a dependency): no
  • The public API: no
  • The schema: no
  • The default values of configurations: no
  • The wire protocol: no
  • The rest endpoints: no
  • The admin cli options: no
  • Anything that affects deployment: don't know

Documentation

  • Does this pull request introduce a new feature: yes
  • Is the feature documented: not documented

I didn't find a documentation section for sink [Record] interface

Copy link
Contributor

@codelipenghui codelipenghui left a comment

Choose a reason for hiding this comment

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

Would you please help add documentation for the new feature at https://pulsar.apache.org/docs/en/io-elasticsearch-sink/

private CredentialsProvider credentialsProvider;
private ElasticSearchConfig elasticSearchConfig;

protected static final String ACTION = "ACTION";
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to add a prefix for the property name e.g. (ELASTCSEARCH_ACTION).

Copy link
Author

Choose a reason for hiding this comment

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

Sure, will do

@alim-zanibekov
Copy link
Author

Would you please help add documentation for the new feature at https://pulsar.apache.org/docs/en/io-elasticsearch-sink/

Yes, but I will really need an example of documentation related to message Schema in topic, e.g. there is no additional documentation about the almost identical feature https://github.com/apache/pulsar/blob/master/site2/docs/io-jdbc-sink.md, sources of this feature - https://github.com/apache/pulsar/blob/master/pulsar-io/jdbc/core/src/main/java/org/apache/pulsar/io/jdbc/JdbcAbstractSink.java

If there is no such an example of documentation, I can make it up by myself. But I'm not well familiar with the Pulsar terminology

@sijie
Copy link
Member

sijie commented Jul 2, 2020

@codelipenghui can you review this again?

@sijie sijie removed this from the 2.7.0 milestone Nov 12, 2020
@eolivelli
Copy link
Contributor

This patch is probably superseded by the recent changes in the ElasticSearch connector.

@github-actions
Copy link

github-actions bot commented Mar 4, 2022

@alim-zanibekov:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@tisonkun
Copy link
Member

tisonkun commented Dec 6, 2022

Closed as stale and conflict. Please rebase and submit a new patch if it's still relevant.

@tisonkun tisonkun closed this Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants