这是indexloc提供的服务,不要输入任何密码
Skip to content
This repository was archived by the owner on Feb 13, 2025. It is now read-only.

Conversation

@harudark
Copy link
Contributor

Description

This pull request adds ES 7.x support in Bosun.
I submitted #2427 before but that is based on my own go modules migration pull request which is different from what upstream maintainers did. This one is based on upstream go modules migration.

Type of change

From the following, please check the options that are relevant.

  • Bug fix (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)
  • This change requires a documentation update

How has this been tested?

Tested with ES 7.2 and also ES 6.8 to make sure nothing breaks.

  • update the configuration toml file and create some alerts to query ES
  • verify alerts can be triggered and no errors in ES query
  • verify annotate can work

Checklist:

  • This contribution follows the project's code of conduct
  • This contribution follows the project's contributing guidelines
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copy link
Member

@muffix muffix left a comment

Choose a reason for hiding this comment

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

Hi @harudark, thanks a lot for your contribution. 🙌
Looking great so far, just a couple of small questions/comments. Would you mind having a look at them and rebasing your branch on master so that we can get it in?

go.sum Outdated
github.com/aws/aws-sdk-go v0.0.0-20180507225419-00862f899353/go.mod h1:ZRmQr0FajVIyZ4ZzBYKG5P3ZqPz9IHG41ZoMu1ADI3k=
github.com/aws/aws-sdk-go v1.1.33 h1:AebOM+6ucC+bSl36BQrYkwShiqruxxJrZKduAvXSD1A=
github.com/aws/aws-sdk-go v1.1.33/go.mod h1:ZRmQr0FajVIyZ4ZzBYKG5P3ZqPz9IHG41ZoMu1ADI3k=
github.com/aws/aws-sdk-go v1.25.25 h1:j3HLOqcDWjNox1DyvJRs+kVQF42Ghtv6oL6cVBfXS3U=
Copy link
Member

Choose a reason for hiding this comment

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

Did you run go mod tidy against that? It seems like we've got multiple versions in here and tidying up might get rid of the dupes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry about that, I forgot it and will fix it.

Copy link
Member

Choose a reason for hiding this comment

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

Cool, happy to merge this once that's done (and rebased on the latest master). Thanks again for the contribution. 🙌

@svagner
Copy link
Contributor

svagner commented Jan 2, 2020

@harudark Should we close MRs #2426, #2427 and #2442 then?

@harudark
Copy link
Contributor Author

harudark commented Jan 8, 2020

@harudark Should we close MRs #2426, #2427 and #2442 then?

I'll fix issues mentioned in code review and then close those MRs.

@langerma
Copy link

langerma commented Mar 3, 2020

any news on that?

@johnewing1
Copy link
Contributor

johnewing1 commented May 20, 2020

@svagner @harudark
Have you had a chance to make the last few changes to this ?

Copy link
Contributor

@johnewing1 johnewing1 left a comment

Choose a reason for hiding this comment

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

Looks good
Thanks for adding this functionality

@johnewing1 johnewing1 merged commit 8a7cd2d into bosun-monitor:master Jun 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants