-
Notifications
You must be signed in to change notification settings - Fork 96
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
Move aws checks to aws plugin #47
Comments
I just took a look at this. The current version of these two checks work just fine with no AWS integration. The AWS component is an optional way to add in more config json to the check. See https://github.com/sensu-plugins/sensu-plugins-http/blob/master/lib/sensu-plugins-http/common.rb#L21 if it's configured, this will go out to S3, fetch some json, and then merge it with the check config. I don't think we want to move these checks over to the sensu-plugins-aws repo. These are very traditional HTTP checks, just two of them have some extra sauce for being able to include extra config info from S3. |
I think we should remove that functionality as thats quite a bit of extra deps pulled in for arbitrary config injection. This would need to be called out in the changelog with a |
We might need to retain the aws-sdk dependency for V4 signing. I'll dig into this a bit more soon. |
If we want we can still remove it: https://docs.amazonaws.cn/en_us/general/latest/gr/signature-v4-examples.html#signature-v4-examples-ruby alternatively we could try just including the core SDK: https://github.com/aws/aws-sdk-ruby/blob/97b28ccf18558fc908fd56f52741cf3329de9869/gems/aws-sdk-core/lib/aws-sdk-core/plugins/signature_v4.rb which would at least reduce the number of dependencies pulled in. |
A few checks in http use
aws-sdk
to check S3. That functionality should move to the aws plugin to minimize dependencies in the http plugin and keep AWS checks together. The checks should be copied to the aws repo, deprecated here, and then removed in a future major release.The text was updated successfully, but these errors were encountered: