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

Remove filter for progressing events in googlechat #169

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

somtochiama
Copy link
Member

Fixes: #165

Signed-off-by: Somtochi Onyekwere somtochionyekwere@gmail.com

Signed-off-by: Somtochi Onyekwere <somtochionyekwere@gmail.com>
@somtochiama
Copy link
Member Author

@djexp I couldn't see why this was done and it is causing some message to not be sent.

I've had to filter out 'Progressing' events as these seemed to replace the previous message within Google Chat. I couldn't easily find out why. However the output in chat is very similar to Discord so why this system does not display these messages is a mystery?

Perhaps you could explain further or attach some screenshots?

@djexp
Copy link
Contributor

djexp commented Mar 24, 2021

@somtochiama The behaviour I was seeing was that Google Chat would seemingly drop all but the latest message that arrived either 'at the same' or within a very very short window. There is no documentation from Google that I could find explaining any such filtering, and there were no errors or status reported through the API to suggest that any such thing was occurring. This just seemed to fit the circumstances.

At the time, using the system I was integration testing with; the Google Chat notifier, (with the filtering) seemed to produce the same output as the Discord and MS Teams implementations; however I didn't have the knowledge with the wider system to investigate further.

I should add that it did not sit right with me to add the filtering code, but without it an incomplete picture was being presented in Google Chat. Clearly if my view on what an 'incomplete picture' is was wrong then this filter should be removed.

Copy link
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

@stefanprodan stefanprodan added the area/alerting Alerting related issues and PRs label Mar 24, 2021
@stefanprodan stefanprodan merged commit 5d6ec21 into fluxcd:main Mar 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/alerting Alerting related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certain types of notification not sent
3 participants