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

Configured the EventGrid Reaction to use different schemas #138

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ruokun-niu
Copy link
Contributor

Description

Currently, the EventGrid reaction only supports eventgrid topics that use CloudEvents as the schema.

This PR configures the Reaction by adding a new property called eventGridSchema. The default value of this property is set to CloudEvents, which publishes CloudEvents to the EventGrid topic. This property can be set to EventGrid to publish events in the EventGrid schema to the topic.

Type of change

  • This pull request is a minor refactor, code cleanup, test improvement, or other maintenance task and doesn't change the functionality of Drasi (issue link optional).

Fixes: #issue_number

Comment on lines 69 to 71
eventGridSchema:
type: string
default: CloudEvents
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
eventGridSchema:
type: string
default: CloudEvents
eventGridSchema:
type: string
enum:
- "CloudEvents"
- "EventGrid"
default: "CloudEvents"

This will also enable validation of one of these 2 options.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it looks like we are missing the format (packed/unpacked) config value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think maybe in the future we should make this format property a default value in the reaction_provider, so that we dont have to implement this for every reaction

{
_logger.LogError($"Error sending message to Event Grid: {resp.Content.ToString()}");
throw new Exception($"Error sending message to Event Grid: {resp.Content.ToString()}");
if (_eventGridSchema == "cloudevents") {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could store an enum in _eventGridSchema to avoid doing a string comparison for every event.

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