-
Notifications
You must be signed in to change notification settings - Fork 139
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
Index receivers using webhook path as key #506
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.
Would you mind adding some more information about why this change is necessary to the PR description, please?
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.
Did some testing to see if the index gets updated when the token changes and the index gets deleted when the receiver is deleted, and it does. Querying for the old index based webhook path results in 404.
Overall, it looks good to me.
Left one last minor comment.
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.
LGTM
Thanks @aryan9600 🏅
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.
Much less computing and more precise reporting about endpoint availability in minimal LOC of changes, absolute great work @aryan9600 🥇
Use `.status.webhookPath` as a key to index Receivers. Use this key while listing Receivers during the handling of a payload. Signed-off-by: Sanskar Jaiswal <[email protected]>
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 putting the icing on the 🍰
Use
.status.webhookPath
as a key to index Receivers. Use this key while listing Receivers during the handling of a payload.This enables us to list only the Receivers required instead of listing all the Receivers in the entire cluster.