Skip to content
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

add checkpointStrategy parameter to event-hub #429

Merged
merged 2 commits into from
May 5, 2021

Conversation

christle
Copy link
Contributor

Signed-off-by: Christian Leinweber christian.leinweber@maibornwolff.de

Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

What do you think @zroubalik ?

content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
Copy link
Member

@zroubalik zroubalik left a comment

Choose a reason for hiding this comment

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

Generally looking good, I have added a minor corrections wrt formatting.
And I'd reformat the checkpointStrategy options to start with lowercase, so it is consistent with the rest of the spec.

content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
Signed-off-by: Christian Leinweber <christian.leinweber@maibornwolff.de>
@christle
Copy link
Contributor Author

christle commented May 4, 2021

I've add more details to the parameter list.

additionally i found out there are also 2 implementations for C#. I have tested both, and the behavior is like Python and Java. One Implementation based on the EventProcessorHost and a new one, which is called EventProcessorClient. The EventProcessorHost works with the default path(no checkpointStrategy specified), the EventProcessorClient with the blobMetadata strategy (which is great, i think). Microsoft called the EventProcessorHost implementation "legacy" (see: https://docs.microsoft.com/de-de/azure/event-hubs/event-hubs-dotnet-standard-get-started-send-legacy).

Copy link
Member

@tomkerkhove tomkerkhove left a comment

Choose a reason for hiding this comment

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

LGTM, just a few tweaks!

content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Show resolved Hide resolved
content/docs/2.3/scalers/azure-event-hub.md Outdated Show resolved Hide resolved
…on event-hub

Signed-off-by: Christian Leinweber <christian.leinweber@maibornwolff.de>
@tomkerkhove tomkerkhove merged commit b08f443 into kedacore:master May 5, 2021
@tomkerkhove
Copy link
Member

Thank you!

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.

3 participants