-
Notifications
You must be signed in to change notification settings - Fork 13
fix: fix CostReport Date LogIc & add status EXPIRED and some logic #402
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
Conversation
Signed-off-by: jinyoungmoonDEV <moonjinyoung.dev@gmail.com>
…me logic Signed-off-by: jinyoungmoonDEV <moonjinyoung.dev@gmail.com>
Signed-off-by: jinyoungmoonDEV <moonjinyoung.dev@gmail.com>
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.
Pull Request Overview
This PR refines the cost report scheduling logic, introduces an EXPIRED status for old reports, and adjusts filtering/deletion/update flows to handle expiration.
- Revise
_calculate_report_flags_and_report_monthsignature and simplify date logic - Add
_change_status_to_expiredmethod and hook it into the persistence flow - Expand status enums and filters to include the new EXPIRED state
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/spaceone/cost_analysis/service/cost_report_serivce.py | Fix date-calculation parameters, add EXPIRED transition logic, remove unused code and adjust delete filters |
| src/spaceone/cost_analysis/model/cost_report/request.py | Extend Status literal to include "EXPIRED" |
| src/spaceone/cost_analysis/model/cost_report/database.py | Update status field choices to allow "EXPIRED" |
Comments suppressed due to low confidence (2)
src/spaceone/cost_analysis/service/cost_report_serivce.py:340
- [nitpick] Remove this stale commented filter since it's been replaced by the combined
"status": ["DONE","EXPIRED"]rule.
# {"k": "status", "v": "DONE", "o": "not"},
src/spaceone/cost_analysis/model/cost_report/request.py:12
- Add or update unit tests to cover transitions into and out of the new
EXPIREDstatus to ensure correct behavior.
Status = Literal["IN_PROGRESS", "ADJUSTING", "DONE", "EXPIRED"]
| self.send_cost_report(cost_report_vo) | ||
|
|
||
| if self._check_done_cost_report_exist(domain_id, cost_report_config_id, report_month): | ||
| self._change_status_to_expired(domain_id, cost_report_config_id, report_month) |
Copilot
AI
Jun 13, 2025
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 argument order for _change_status_to_expired doesn’t match its signature (report_month, domain_id, cost_report_config_id). Swap them to (report_month, domain_id, cost_report_config_id).
| self._change_status_to_expired(domain_id, cost_report_config_id, report_month) | |
| self._change_status_to_expired(report_month, domain_id, cost_report_config_id) |
| cost_report_update_query | ||
| ) | ||
|
|
||
| for cost_report_vo in cost_reports_vos: |
Copilot
AI
Jun 13, 2025
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.
Iterating and updating each report one by one may be inefficient for large datasets; consider using a bulk update query if supported by the manager.
Category
Description