-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feat daily cost notification template #69
Conversation
Warning Rate Limit Exceeded@nao1215 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 24 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe recent update introduces a comprehensive Daily Cost Notification feature across the application. It leverages AWS services like Cost Explorer, SNS, and Lambda to provide users with daily updates on their AWS spending. The implementation covers various aspects, from domain logic to external service interactions and cloud infrastructure deployment, ensuring thorough documentation and testing. Changes
Related issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@@ -0,0 +1,15 @@ | |||
package mock |
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.
🚫 [golangci] reported by reviewdog 🐶
ST1000: at least one file in a package should have a package comment (stylecheck)
app/interactor/cost_explorer.go
Outdated
// GetCost gets the cost. | ||
func (c *CostGetter) GetCost(ctx context.Context, input *usecase.CostGetterInput) (*usecase.CostGetterOutput, error) { | ||
if input.End.Before(input.Start) { | ||
return nil, errors.New("End date is before the start date") |
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.
🚫 [golangci] reported by reviewdog 🐶
ST1005: error strings should not be capitalized (stylecheck)
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (6)
cloudformation/daily-cost-notification/daily-cost-email.png
is excluded by:!**/*.png
cloudformation/daily-cost-notification/daily-cost-notification.png
is excluded by:!**/*.png
cloudformation/daily-cost-notification/subscription-confirm.png
is excluded by:!**/*.png
cloudformation/daily-cost-notification/template.yml
is excluded by:!**/*.yml
go.mod
is excluded by:!**/*.mod
go.sum
is excluded by:!**/*.sum
Files selected for processing (19)
- README.md (1 hunks)
- app/domain/service/cost_explorer.go (1 hunks)
- app/domain/service/sns.go (1 hunks)
- app/external/cost_explorer.go (1 hunks)
- app/external/mock/cost_explorer.go (1 hunks)
- app/external/mock/sns.go (1 hunks)
- app/external/sns.go (1 hunks)
- app/interactor/cost_explorer.go (1 hunks)
- app/interactor/cost_explorer_test.go (1 hunks)
- app/interactor/sns.go (1 hunks)
- app/interactor/sns_test.go (1 hunks)
- app/usecase/cost_explorer.go (1 hunks)
- app/usecase/sns.go (1 hunks)
- cloudformation/Makefile (1 hunks)
- cloudformation/daily-cost-notification/.gitignore (1 hunks)
- cloudformation/daily-cost-notification/Makefile (1 hunks)
- cloudformation/daily-cost-notification/README.md (1 hunks)
- cloudformation/daily-cost-notification/daily-cost-notification.drawio (1 hunks)
- cloudformation/daily-cost-notification/main.go (1 hunks)
Files skipped from review due to trivial changes (1)
- cloudformation/daily-cost-notification/.gitignore
Additional comments: 5
app/external/mock/cost_explorer.go (1)
- 1-15: The mock implementation of
CostGetter
is correctly defined and follows best practices for mocking in Go. It allows for flexible testing scenarios by using a function type.app/external/mock/sns.go (1)
- 1-15: The mock implementation of
SNSPublisher
is correctly defined and follows the established pattern for mocking in Go, ensuring consistency and flexibility in testing.app/domain/service/sns.go (1)
- 1-19: The
SNSPublisher
interface and its associated types are well-defined, following best practices for interface design in Go. The use of context and clear method signatures ensures a good foundation for implementing SNS publishing functionality.README.md (1)
- 50-50: The addition of the "Daily Cost Notification" feature to the Template List is clear and concise, effectively informing users about the new feature's availability and its documentation. This update integrates well with the existing structure of the README.md file, maintaining consistency in presentation and formatting.
cloudformation/daily-cost-notification/daily-cost-notification.drawio (1)
- 1-69: The diagram provided in
daily-cost-notification.drawio
effectively illustrates the workflow of the "Daily Cost Notification" feature, using clear and accurate representations of AWS services and the interactions between them. The logical flow from scheduled execution through EventBridge, processing by a Lambda function, logging via CloudWatch, and finally publishing a notification through SNS to send an email is well depicted. This diagram will be valuable for users and developers to understand the feature's architecture and operational dynamics.
package usecase | ||
|
||
import "context" | ||
|
||
// SNSPublisherInput is the input of the Publish method. | ||
type SNSPublisherInput struct { | ||
// TopicArn is the ARN of the topic. | ||
TopicArn string | ||
// Message is the message that you want to publish. | ||
Message string | ||
} | ||
|
||
// SNSPublisherOutput is the output of the Publish method. | ||
type SNSPublisherOutput struct{} | ||
|
||
// SNSPublisher is the interface that wraps the basic Publish method. | ||
type SNSPublisher interface { | ||
PublishSNS(ctx context.Context, input *SNSPublisherInput) (*SNSPublisherOutput, error) | ||
} |
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.
The SNSPublisher
interface and its associated types are duplicated here and in app/domain/service/sns.go
. To maintain clarity and reduce redundancy, it's recommended to remove these duplicate definitions and rely on the ones in the domain layer.
- // SNSPublisherInput is the input of the Publish method.
- type SNSPublisherInput struct {
- // TopicArn is the ARN of the topic.
- TopicArn string
- // Message is the message that you want to publish.
- Message string
- }
-
- // SNSPublisherOutput is the output of the Publish method.
- type SNSPublisherOutput struct{}
-
- // SNSPublisher is the interface that wraps the basic Publish method.
- type SNSPublisher interface {
- PublishSNS(ctx context.Context, input *SNSPublisherInput) (*SNSPublisherOutput, error)
- }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
package usecase | |
import "context" | |
// SNSPublisherInput is the input of the Publish method. | |
type SNSPublisherInput struct { | |
// TopicArn is the ARN of the topic. | |
TopicArn string | |
// Message is the message that you want to publish. | |
Message string | |
} | |
// SNSPublisherOutput is the output of the Publish method. | |
type SNSPublisherOutput struct{} | |
// SNSPublisher is the interface that wraps the basic Publish method. | |
type SNSPublisher interface { | |
PublishSNS(ctx context.Context, input *SNSPublisherInput) (*SNSPublisherOutput, error) | |
} | |
package usecase | |
import "context" |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to path filters (2)
go.mod
is excluded by:!**/*.mod
go.sum
is excluded by:!**/*.sum
Files selected for processing (4)
- .github/workflows/cfn-guard.yml (1 hunks)
- .github/workflows/cloudformation.yml (1 hunks)
- app/external/mock/cost_explorer.go (1 hunks)
- app/interactor/cost_explorer.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- app/external/mock/cost_explorer.go
- app/interactor/cost_explorer.go
.github/workflows/cfn-guard.yml
Outdated
@@ -4,6 +4,7 @@ on: | |||
push: | |||
paths: | |||
- 'cloudformation/**' | |||
- 'cloudformation/**/**' |
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.
The path pattern 'cloudformation/**/**'
is redundant because the glob pattern 'cloudformation/**'
already matches files in all directories and subdirectories under 'cloudformation'
. Consider removing the redundant pattern to simplify the workflow configuration.
- - 'cloudformation/**/**'
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
- 'cloudformation/**/**' |
This comment has been minimized.
This comment has been minimized.
HOTTEST report
Reported by hottest |
Code Metrics Report
Details | | main (d5de19e) | #69 (70927ad) | +/- |
|---------------------|----------------|---------------|-------|
- | Coverage | 22.3% | 22.2% | -0.0% |
| Files | 53 | 60 | +7 |
| Lines | 1835 | 1893 | +58 |
+ | Covered | 409 | 421 | +12 |
+ | Test Execution Time | 43s | 11s | -32s | Code coverage of files in pull request scope (0.0% → 20.7%)
Reported by octocov |
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
Refactor