Skip to content

[Pull Request] Merged Refactoring codes to Master #359

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

Open
wants to merge 85 commits into
base: feature-refactoring
Choose a base branch
from

Conversation

jinyoungmoonDEV
Copy link
Contributor

Category

  • New feature
  • Bug fix
  • Improvement
  • Refactor
  • etc

Description

[Pull Request] Merged Refactoring codes to Master

ImMin5 and others added 30 commits March 11, 2025 20:01
Modify analyze Daily date range 1 month -> 31days
Modify budget and budget usages all apis
Add convert model decorator at budget usage analyze api
Add budget alert notification function
Signed-off-by: ImMin5 <[email protected]>
Add logic for project based budget
# Conflicts:
#	src/spaceone/cost_analysis/interface/grpc/budget_usage.py
#	src/spaceone/cost_analysis/manager/budget_manager.py
#	src/spaceone/cost_analysis/manager/budget_usage_manager.py
#	src/spaceone/cost_analysis/manager/email_manager.py
#	src/spaceone/cost_analysis/manager/identity_manager.py
#	src/spaceone/cost_analysis/model/__init__.py
#	src/spaceone/cost_analysis/model/budget/request.py
#	src/spaceone/cost_analysis/model/budget/response.py
#	src/spaceone/cost_analysis/model/budget_usage/request.py
#	src/spaceone/cost_analysis/model/budget_usage/response.py
#	src/spaceone/cost_analysis/service/budget_service.py
#	src/spaceone/cost_analysis/service/budget_usage_service.py
#	src/spaceone/cost_analysis/service/cost_report_serivce.py
#	src/spaceone/cost_analysis/service/unified_cost_service.py
Modify field name notifications -> notification
ImMin5 and others added 17 commits April 22, 2025 14:35
Implement budget state update job scheduling and enhance budget state management
Add service_account_id field to budget usage model
Add service_account_id field to budget usage list request query filter
Migrate cost report services to UnifiedCost model
…ster

Revert "Migrate cost report services to UnifiedCost model"
…-pg-filter

Add Unified cost analysis with domain_id filtering and project_group_id
Add error handling for duplicated budget thresholds
Update workspace_id field to be optional when get budget info
# Conflicts:
#	src/spaceone/cost_analysis/manager/budget_manager.py
#	src/spaceone/cost_analysis/manager/budget_usage_manager.py
#	src/spaceone/cost_analysis/service/budget_service.py
@jinyoungmoonDEV jinyoungmoonDEV requested a review from ImMin5 April 29, 2025 02:21
Copy link

⚠️ @jinyoungmoonDEV the signed-off-by was not found in the following 14 commits:

✅ Why it is required

The Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO.

Contributors sign-off that they adhere to these requirements by adding a Signed-off-by line to commit messages.

This is my commit message

Signed-off-by: Random Developer <[email protected]>

Git even has a -s command line option to append this automatically to your commit message:

$ git commit -s -m 'This is my commit message'

Copy link

⚠️ @jinyoungmoonDEV the signed-off-by was not found in the following 14 commits:

✅ Why it is required

The Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO.

Contributors sign-off that they adhere to these requirements by adding a Signed-off-by line to commit messages.

This is my commit message

Signed-off-by: Random Developer <[email protected]>

Git even has a -s command line option to append this automatically to your commit message:

$ git commit -s -m 'This is my commit message'

Copy link
Member

@ImMin5 ImMin5 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 overall, just a minor tweak needed for the type hint.

@@ -85,7 +85,8 @@ def get_data_source_plugin_endpoint_by_vo(self, data_source_vo: DataSource):

return endpoint

def get_data_source_plugin_endpoint(self, plugin_info, domain_id):
@staticmethod
def get_data_source_plugin_endpoint(plugin_info, domain_id):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def get_data_source_plugin_endpoint(plugin_info, domain_id):
def get_data_source_plugin_endpoint(plugin_info: dict, domain_id:str) -> str:

@@ -113,7 +114,8 @@ def upgrade_data_source_plugin_version(
plugin_metadata, resource_group, data_source_id, workspace_id, domain_id
)

def delete_data_source_rules(self, data_source_id, domain_id):
@staticmethod
def delete_data_source_rules(data_source_id, domain_id):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def delete_data_source_rules(data_source_id, domain_id):
def delete_data_source_rules(data_source_id: str, domain_id:str ) -> None:

@@ -73,7 +73,8 @@ def create_job(
_LOGGER.debug(f"[create_job] create job: {job_vo.job_id}")
return job_vo

def update_job_by_vo(self, params, job_vo):
@staticmethod
def update_job_by_vo(params, job_vo):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def update_job_by_vo(params, job_vo):
def update_job_by_vo(params: dict, job_vo: Job) -> Job:

@@ -26,7 +26,7 @@ class DataSourceRuleCreateRequest(BaseModel):
options: Union[dict, None] = None
tags: Union[dict, None] = None
resource_group: ResourceGroup
workspace_id: str
workspace_id: Union[dict, None] = None
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
workspace_id: Union[dict, None] = None
workspace_id: Union[str, None] = None

@ImMin5 ImMin5 requested a review from Copilot April 30, 2025 04:27
@ImMin5 ImMin5 added the enhancement New feature or request label Apr 30, 2025
Copy link

@Copilot Copilot AI left a 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 pull request merges extensive refactoring changes into master for the cost analysis module. Key changes include updates to date range logic in cost management, reordering and renaming of parameters in budget usage and notification methods, and adjustments in scheduler and error message configurations.

Reviewed Changes

Copilot reviewed 55 out of 55 changed files in this pull request and generated 2 comments.

File Description
src/spaceone/cost_analysis/manager/cost_manager.py Changed date range check from 1 month to 31 days and updated the corresponding error message.
src/spaceone/cost_analysis/manager/budget_usage_manager.py Reordered arguments in the _update_monthly_budget_usage call and updated budget usage notification logic.
src/spaceone/cost_analysis/manager/budget_manager.py Minor adjustments including default tag assignment and updated method signatures.
Various scheduler and configuration files Updated scheduler instantiation, configuration settings, and deployment files as part of the refactor.

Comment on lines +305 to +309
if start + relativedelta(days=31) < end:
raise ERROR_INVALID_DATE_RANGE(
start=start_str,
end=end_str,
reason="Request up to a maximum of 1 month.",
reason="Request up to a maximum of 1 month. or 31 days.",
Copy link
Preview

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

Changing the date range check from 1 month to 31 days may introduce edge cases for months with fewer or more days. Ensure this change fully aligns with the intended business logic.

Copilot uses AI. Check for mistakes.

_LOGGER.info(f"[update_cost_usage] Update Budget Usage: {budget_vo.budget_id}")
unified_cost_mgr = UnifiedCostManager()

self._update_monthly_budget_usage(budget_vo, unified_cost_mgr)
self._update_monthly_budget_usage(unified_cost_mgr, budget_vo)
Copy link
Preview

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

The argument order for _update_monthly_budget_usage has been changed; verify that the new order is consistent with its definition and that all callers have been updated accordingly.

Suggested change
self._update_monthly_budget_usage(unified_cost_mgr, budget_vo)
self._update_monthly_budget_usage(budget_vo, unified_cost_mgr)

Copilot uses AI. Check for mistakes.

Copy link

github-actions bot commented May 7, 2025

⚠️ @jinyoungmoonDEV the signed-off-by was not found in the following 14 commits:

✅ Why it is required

The Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO.

Contributors sign-off that they adhere to these requirements by adding a Signed-off-by line to commit messages.

This is my commit message

Signed-off-by: Random Developer <[email protected]>

Git even has a -s command line option to append this automatically to your commit message:

$ git commit -s -m 'This is my commit message'

1 similar comment
Copy link

github-actions bot commented May 7, 2025

⚠️ @jinyoungmoonDEV the signed-off-by was not found in the following 14 commits:

✅ Why it is required

The Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO.

Contributors sign-off that they adhere to these requirements by adding a Signed-off-by line to commit messages.

This is my commit message

Signed-off-by: Random Developer <[email protected]>

Git even has a -s command line option to append this automatically to your commit message:

$ git commit -s -m 'This is my commit message'

@jinyoungmoonDEV jinyoungmoonDEV requested a review from ImMin5 May 7, 2025 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fail/signedoff
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants