-
Notifications
You must be signed in to change notification settings - Fork 84
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
TT-10675 add SQS Pump Backend support #740
TT-10675 add SQS Pump Backend support #740
Conversation
Apply Sweep Rules to your PR?
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @masoudhaghbin! Thank you for your contribution, we really appreciate it!
Overall, it looks pretty good. I just left 2 small suggestions.
The only missing points are:
- Add this new pump to the readme (you can follow this example of another pump)
- Add unit tests for this new pump (you can follow this unit test file example of another pump)
We're looking forward to merging this PR once the suggested changes are addressed. Thank you for your patience!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
@masoudhaghbin I created a duplicated version of this PR with all your commits so CIs can effectively run without complaining about some repo secrets. I'll close this one so we can keep working on the new one and merge it. You can take a look at it here. Again, thank you for your contribution! |
Add SQS pump to supported backends
Description
The PR will add the support to pump logs into a SQS instance
Related Issue
#737
Motivation and Context
How This Has Been Tested
Screenshots (if appropriate)
Types of changes
Checklist
fork, don't request your
master
!master
branch (left side). Also, you should startyour branch off our latest
master
.go mod tidy && go mod vendor
go fmt -s
go vet