Skip to content

Comments

support modern and legacy elasticsearch clients#132

Draft
missinglink wants to merge 1 commit intomasterfrom
v2-client
Draft

support modern and legacy elasticsearch clients#132
missinglink wants to merge 1 commit intomasterfrom
v2-client

Conversation

@missinglink
Copy link
Member

this is a proof-of-concept for supporting both the legacy elasticsearch client and the modern client.

it would be a breaking change to this repo but would allow us a path to migrate incrementally without breaking changes to the config.

@missinglink missinglink requested a review from orangejulius June 19, 2025 14:28
@missinglink
Copy link
Member Author

note: I haven't tested the v2/sink implementation, it likely requires some tweaking to work correctly, in particular the onDocument callback might need some modifications as I believe the bulk API is slightly different than the regular indexing API?

https://www.elastic.co/docs/reference/elasticsearch/clients/javascript/client-helpers

Comment on lines +5 to +14
module.exports = {
v1: {
client: require('./src/client'),
createWriteStream: require('./src/sink')
},
v2: {
client: require('./v2/client'),
createWriteStream: require('./v2/sink')
}
};
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be a breaking change to the interface of the dbclient module, meaning we'll need changes in all importers. Is that intended?

If so can you provide some instructions on how to test this PR as it stands?

Copy link
Member Author

Choose a reason for hiding this comment

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

unfortunately I believe this is unavoidable due to the current export being on the root?

module.exports = require('./src/sink');

Copy link
Member Author

@missinglink missinglink Sep 22, 2025

Choose a reason for hiding this comment

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

We could probably handle it as a breaking change, then migrate all importers to use the v1.createWriteStream path instead of the module root.

I'm open to ideas, just working around the limitations we have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants