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

Counter Query Monitor (Take 2) #18

Merged
merged 15 commits into from
Aug 29, 2022
Merged

Counter Query Monitor (Take 2) #18

merged 15 commits into from
Aug 29, 2022

Conversation

bh2smith
Copy link
Contributor

The Counter Query Monitor expects the results to have only a single row, one can specify a column of that row and a threshold for the value.

Use Cases:

  1. Monitoring to see when an account balance falls below a certain value.
  2. Be alerted when our trading volume exceeds 100B so we can tweet about it.

Not much different from #16 but now based on #17 instead, should demonstrate how it is now super easy to add a new type of query monitor.

I noticed here that I wasn't properly using abstract method decorators in the base class which only became apparent here. This could be moved into the base PR.

Test Plan

New and existing tests.

Copy link

@gentrexha gentrexha left a comment

Choose a reason for hiding this comment

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

Good to know it's so easy to add new types of query monitors in the future. LGTM!

Copy link

@josojo josojo left a comment

Choose a reason for hiding this comment

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

Nice

Base automatically changed from remodeling-structures to main August 29, 2022 09:58
@bh2smith bh2smith merged commit 02fd417 into main Aug 29, 2022
@bh2smith bh2smith deleted the counter-take2 branch August 29, 2022 10:07
@bh2smith bh2smith mentioned this pull request Aug 30, 2022
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