-
Notifications
You must be signed in to change notification settings - Fork 554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Export missing records to ES #10697
Export missing records to ES #10697
Conversation
Hi @remcowesterhoud. Please take a moment to check this out. 🙇 |
@remcowesterhoud good point. 👍 Yes, let's export these records by default. 🚀 Except, the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for another contribution @lzgabel 🏅
The indexes themselves look good. There is a few other things that require some attention though.
- This wasn't in the issue so you couldn't have known, but please change exporting of the
timer
,message_start_event_subscription
anddeployment_distribution
totrue
by default. - We'll have to add these records to the ElasticSearchExporter
- We should modify the toString() method of the
ElasticSearchExporterConfiguration
. - We have some parametrised tests these tests use this TestSupport class. We will have to add the value types of these records to the
setIndexingForValueType()
method and remove them from theprovideValueTypes()
method.
If you will, please make these changes in a separate commit. This makes it easier for me as I'll only have to review the new commit 🙂
Hi @remcowesterhoud. Please check this out. 🙇 |
You are lightning fast ⚡ I will have a look tomorrow! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome! 🏆
The issue is not marked as hacktoberfest, but I reckon it still deserves it so I'll add the hacktoberfest-accepted
label to this PR. If you have 4 PRs within Camunda labelled with hacktoberfest-accepted
you have completed the hacktoberfest challenge. In this case please submit the entry form here 🙂
bors merge
Hi @remcowesterhoud. Seems like |
@lzgabel Yes, please do a rebase |
Export some records by default, and add validation tests.
bors cancel |
fef28f6
to
65f3812
Compare
Done. |
bors merge |
Build succeeded: |
Description
Currently, the Zeebe Elasticsearch exporter doesn't allow to export all records. The following records are missing:
timer
message_start_event_subscription
process_event
deployment_distribution
Related issues
closes #8337
Definition of Done
Code changes:
backport stable/1.3
) to the PR, in case that fails you need to create backports manually.Testing:
Documentation:
Please refer to our review guidelines.