Updated to trigger a notification if CPU recommendations are unavailable at profile level. #1156
Conversation
…ble at profile level. Signed-off-by: msvinaykumar <vinakuma@redhat.com>
There was a problem hiding this comment.
LGTM
But I feel we need to have unified notification structure in future.
I see engineNotifications object is created at same level and notifications are populated to that and at the end they are being added to recommendationModel if we have notifications object passed we should make use of the same object instead of creating a new one.
|
@msvinaykumar - Can you please update the function test suite with tests to ensure such regressions are not missed out. You can refer this issue to check for tests added for which notifications. Thank you. |
@chandrams was there any previously written test cases check for CPU usage being zero or less than a millicore. Or Do I need to include new test cases? |
|
@msvinaykumar - Tests for these notifications aren't present in the testsuite, new tests are required. You can check the issue above to see for which notification codes tests were added earlier. |
| //Set all existing notifications | ||
| for (RecommendationNotification recommendationNotification : notifications) { | ||
| recommendationModel.addNotification(recommendationNotification); | ||
| } | ||
|
|
There was a problem hiding this comment.
Is this the right place to add these notifications? Can you please paste an example JSON object that has this notification
There was a problem hiding this comment.
The reason notifications aren't being set is because the function checks if the flag isRecommendedCPURequestAvailable is set to false. In such cases, these notifications are lost, despite the existing code intending to set a notification here.
https://privatebin.corp.redhat.com/?0252306990b9c2fc#5KA1424pqZhQbbogX7uD8ho36VJk5jngts69uU9F4jYV
Signed-off-by: msvinaykumar <vinakuma@redhat.com>
done |
…e and memory zero Signed-off-by: msvinaykumar <vinakuma@redhat.com>
Signed-off-by: msvinaykumar <vinakuma@redhat.com>
Signed-off-by: msvinaykumar <vinakuma@redhat.com>
Updated to trigger a notification if CPU recommendations are unavailable at profile level.
following notifications are missed
"CPU Usage is less than a millicore, No CPU Recommendations can be generated"
"CPU usage is zero, No CPU Recommendations can be generated"