[COST-6424] Add option for OCP ROS reports only#622
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a --ros-only flag to the OCP report generation process, allowing for the exclusive generation of ROS-specific data. The implementation includes updates to the CLI parser, the OCPGenerator class, and the report creation logic. Feedback suggests refactoring the initialization of report configuration dictionaries in ocp_generator.py and report.py to eliminate code duplication and improve maintainability.
| if ros_only: | ||
| # Only ROS reports | ||
| data = { | ||
| OCP_ROS_USAGE: [], | ||
| OCP_ROS_NAMESPACE_USAGE: [], | ||
| } | ||
| file_numbers = { | ||
| OCP_ROS_USAGE: 0, | ||
| OCP_ROS_NAMESPACE_USAGE: 0, | ||
| } | ||
| else: | ||
| data = { | ||
| OCP_POD_USAGE: [], | ||
| OCP_STORAGE_USAGE: [], | ||
| OCP_NODE_LABEL: [], | ||
| OCP_NAMESPACE_LABEL: [], | ||
| OCP_VM_USAGE: [], | ||
| OCP_GPU_USAGE: [], | ||
| } | ||
| file_numbers = { | ||
| OCP_POD_USAGE: 0, | ||
| OCP_STORAGE_USAGE: 0, | ||
| OCP_NODE_LABEL: 0, | ||
| OCP_NAMESPACE_LABEL: 0, | ||
| OCP_VM_USAGE: 0, | ||
| OCP_GPU_USAGE: 0, | ||
| } | ||
| if ros_ocp_info: | ||
| data.update({OCP_ROS_USAGE: [], OCP_ROS_NAMESPACE_USAGE: []}) | ||
| file_numbers.update({OCP_ROS_USAGE: 0, OCP_ROS_NAMESPACE_USAGE: 0}) |
There was a problem hiding this comment.
The initialization of the data and file_numbers dictionaries is duplicated across the ros_only and ros_ocp_info logic paths. This can be simplified by initializing the base dictionaries and conditionally updating them with ROS-specific keys, making the code more concise and easier to maintain.
data = {}
file_numbers = {}
if not ros_only:
data.update({
OCP_POD_USAGE: [],
OCP_STORAGE_USAGE: [],
OCP_NODE_LABEL: [],
OCP_NAMESPACE_LABEL: [],
OCP_VM_USAGE: [],
OCP_GPU_USAGE: [],
})
file_numbers.update({
OCP_POD_USAGE: 0,
OCP_STORAGE_USAGE: 0,
OCP_NODE_LABEL: 0,
OCP_NAMESPACE_LABEL: 0,
OCP_VM_USAGE: 0,
OCP_GPU_USAGE: 0,
})
if ros_only or ros_ocp_info:
ros_keys = [OCP_ROS_USAGE, OCP_ROS_NAMESPACE_USAGE]
data.update({k: [] for k in ros_keys})
file_numbers.update({k: 0 for k in ros_keys})
There was a problem hiding this comment.
suggestion: @dchorvat1 the data/file_numbers init (lines 919-948) can reuse the existing COST_OCP_REPORT_TYPE_TO_COLS, ROS_OCP_REPORT_TYPE_TO_COLS, and OCP_REPORT_TYPE_TO_COLS dicts from ocp_generator.py, since their keys are exactly the report types being toggled:
if ros_only:
report_types = ROS_OCP_REPORT_TYPE_TO_COLS
elif ros_ocp_info:
report_types = OCP_REPORT_TYPE_TO_COLS
else:
report_types = COST_OCP_REPORT_TYPE_TO_COLS
data = {rt: [] for rt in report_types}
file_numbers = {rt: 0 for rt in report_types}
this eliminates the duplication and means future report type additions only need updating in one place. just needs those dicts exported from ocp/init.py.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #622 +/- ##
=======================================
- Coverage 93.6% 93.5% -0.1%
=======================================
Files 56 56
Lines 4794 4800 +6
Branches 672 674 +2
=======================================
+ Hits 4488 4490 +2
- Misses 165 167 +2
- Partials 141 143 +2 🚀 New features to boost your workflow:
|
2 participants
COST-6424
Adds capability for generation of only ROS reports for OCP.
Tested with following commands:
The commands returned expected results, with first one returning OCP reports without ROS data, second one returning OCP data with ROS data and third one returning ROS data only.
Also verified against local koku with OCP raw calc test (passed)