FLPATH-2749 | Refactor the Plugin code based on new structure to cost-management#2341
FLPATH-2749 | Refactor the Plugin code based on new structure to cost-management#2341asmasarw wants to merge 1 commit intoredhat-developer:mainfrom
Conversation
asmasarw
requested review from
PreetiW,
batzionb,
mareklibra and
ydayagi
as code owners
Missing ChangesetsThe following package(s) are changed by this PR but do not have a changeset:
See CONTRIBUTING.md for more information about how to add changesets. Changed Packages
|
asmasarw
changed the title
asmasarw
force-pushed
the
feature/cost-m
branch
from
d7564fa to
17df515
Compare
| credentials: dangerously-allow-unauthenticated | ||
| resourceOptimization: | ||
| costManagement: | ||
| clientId: ${ROS_CLIENT_ID} |
There was a problem hiding this comment.
@asmasarw if it is called costManagement, why do we still have env variables with ROS_ shouldn't it be refactored too?
There was a problem hiding this comment.
agree,
changed to CM_
| const baseUrl = | ||
| configApi.getOptionalString('costManagementProxyBaseUrl') ?? | ||
| configApi.getOptionalString( | ||
| 'costManagement.costManagementProxyBaseUrl', |
There was a problem hiding this comment.
@asmasarw this seem to be a breaking change, you added a prefix.
There was a problem hiding this comment.
This was done by mistake, Reverted.
Thanks
| 'costManagement.costManagementProxyBaseUrl', | ||
| ) ?? | ||
| configApi.getOptionalString( | ||
| 'costManagement.optimizationsBaseUrl', |
There was a problem hiding this comment.
@asmasarw here too, the prefix was added not refactored.
There was a problem hiding this comment.
This was done by mistake, Reverted.
Thanks
|
|
||
| export interface Config { | ||
| resourceOptimization: { | ||
| costManagement: { |
There was a problem hiding this comment.
As I commented before, previously this was used without prefix, and after the refactor you introduced a prefix, is thsi expect? is this the config?
There was a problem hiding this comment.
Here the name is fine, but the nested values were incorrect.
Reverted.
| * Override generated pluginId so discovery matches the backend plugin ID | ||
| * (OpenAPI info.title is set to 'cost-management' for the API name). | ||
| */ | ||
| setPluginIdValue(pluginIdFilePath, value) { |
There was a problem hiding this comment.
Why do we need this? If this is a refactoring PR, why do we have new code. I don't understand why is this relevant and previously wasnt.
There was a problem hiding this comment.
This was done by mistake, Reverted.
Thanks
| console.log('Setting pluginId to backend plugin ID for discovery'); | ||
| tsSourceFileMutator.setPluginIdValue( | ||
| `${commonPackageDir}/src/generated/pluginId.ts`, | ||
| 'redhat-resource-optimization', |
There was a problem hiding this comment.
This was done by mistake, Reverted.
| const result: any = []; | ||
|
|
||
| series.map((s, index) => { | ||
| series.forEach((s, index) => { |
There was a problem hiding this comment.
This is a Sonar Qube Issue fixed here.
CI fail here, unless we're changing the map to forEach.
asmasarw
force-pushed
the
feature/cost-m
branch
from
2c6e89d to
fec97ca
Compare
|
2 participants
FLPATH-2749 | Refactor the Plugin code based on new structure to cost-management