-
Notifications
You must be signed in to change notification settings - Fork 5
EES-5952 Add infrastructure for Event Grid topic subscriptions #5773
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
EES-5952 Add infrastructure for Event Grid topic subscriptions #5773
Conversation
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.
As always, please find some thoughts - but merge with the amendments (if any) you feel fit.
...structure/templates/common/components/event-grid/eventGridCustomTopicQueueSubscription.bicep
Show resolved
Hide resolved
@maxLength(64) | ||
param name string | ||
|
||
@description('The name of the parent topic.') |
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.
Is the word "parent" necessary here? Perhaps "the name of the event grid topic to which this subscription is subscribed" or words to that effect?
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.
No it's not necessary 🙂 . I only used the word "parent" because they refer to it as the parent in some of the descriptions of properties in the Bicep resource template documentation. I've renamed this now to "The Event Grid topic name associated with the subscription."
I also found that my formatter in VS code wasn't set up correctly so I've done a limited amount of reformatting at the same time.
infrastructure/templates/search/application/searchDocsFunctionEventSubscriptions.bicep
Show resolved
Hide resolved
@@ -15,3 +15,14 @@ type ResourceNames = { | |||
@export() | |||
// The built in Search Service roles. See https://learn.microsoft.com/en-gb/azure/role-based-access-control/built-in-roles/ai-machine-learning | |||
type SearchServiceRole = 'Search Index Data Contributor' | 'Search Index Data Reader' | 'Search Service Contributor' | |||
|
|||
@export() | |||
type StorageQueueNamesType = { |
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.
I think, because this is so specific to search, call this type SearchStorageQueueNames
?
Is the Type
suffix a thing in bicep?
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.
No I don't think it's specific to use that as a suffix but a lot of the documentation on custom types has it in examples.
I've removed it for now to make it in keeping with our other custom types, and also renamed it SearchStorageQueueNames
as suggested.
e542289
to
9acaf8a
Compare
This PR follows on from #5768 which added Event Grid topics that the Admin and Publisher will use to publish events when certain conditions occur.
This PR subscribes the Search Function app to these topics.
Because the functions in the Search Function app have been configured to use queue trigger bindings rather than Event Grid trigger bindings, the Event Grid subscriptions are created using the storage queues as their destinations.