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

Region filtering for health notifications #12

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

bjfish25
Copy link
Contributor

@bjfish25 bjfish25 commented Aug 9, 2021

Proposed changes

It is possible for AWS to generate notifications that are unwanted / uncared about as they involve a region not currently in use. Thus we would like a way to filter these out from use.

Issues for these changes

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (changes to code, which do not change application behavior)

Checklist

  • I have filled out this PR template
  • I have read the CONTRIBUTING doc
  • I have added automated tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (README.md, CHANGELOG.md, etc. - if appropriate)

Dependencies and Blockers

Relevant Links

Further comments

@bjfish25 bjfish25 requested review from shubydo and rajholla August 9, 2021 16:42
@github-actions

This comment has been minimized.

Comment on lines +230 to +231
healthCmd.PersistentFlags().StringVar(&healthExcludeRegions, "exclude-regions", "", "Set of regions separated by [,] to exclude Health Notifications from. Takes precedence over include-regions")
healthCmd.PersistentFlags().StringVar(&healthIncludeRegions, "include-regions", "", "Set of regions separated by [,] to include Health Notifications from. Use \"global\" as a region if wanting notifications affecting all regions. Ignored when exclude-regions is set")
Copy link
Collaborator

Choose a reason for hiding this comment

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

If neither include-regions or exclude-regions is specified, will results be returned for every region?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

@bjfish25 bjfish25 requested a review from shubydo August 9, 2021 20:42
@shubydo shubydo merged commit 8bad266 into master Aug 9, 2021
@shubydo shubydo deleted the feature/health-region-filtering branch August 9, 2021 20:54
@github-actions
Copy link

github-actions bot commented Aug 9, 2021

Unit Test Results

    2 files  ±0      2 suites  ±0   10s ⏱️ ±0s
255 tests +2  255 ✔️ +2  0 💤 ±0  0 ❌ ±0 

Results for commit 8bad266. ± Comparison against base commit 65a0201.

@@ -1,3 +1,8 @@
## v0.1.2 ( 9 August 2021)
Copy link
Collaborator

@rajholla rajholla Aug 10, 2021

Choose a reason for hiding this comment

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

new feature - v0.2.0, even otherwise v.0.1.3

@rajholla
Copy link
Collaborator

nicely done!

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