Health summary with clusters and databases #878
Conversation
Co-Authored-By: Eugen Maksymenko <eugen.maksymenko@suse.com>
CDimonaco
added
enhancement
CDimonaco
requested review from
dottorblaster,
fabriziosestito,
nelsonkopliku and
arbulu89
lib/trento/application/usecases/sap_systems/health_summary_service.ex
Outdated
Show resolved
Hide resolved
lib/trento/application/usecases/sap_systems/dto/health_summary_dto.ex
Outdated
Show resolved
Hide resolved
lib/trento/application/usecases/sap_systems/health_summary_service.ex
Outdated
Show resolved
Hide resolved
lib/trento/application/usecases/sap_systems/health_summary_service.ex
Outdated
Show resolved
Hide resolved
lib/trento/application/usecases/sap_systems/health_summary_service.ex
Outdated
Show resolved
Hide resolved
|
Ready to rereview @arbulu89 |
There was a problem hiding this comment.
@CDimonaco the changes look good.
Now I have the doubt if we need to send the database_id, because as far as I can see, they sap_system_id and database_id are the same.
Maybe we can remove this database_id
| %Trento.ClusterReadModel{id: cluster_id} = | ||
| insert(:cluster, type: ClusterType.hana_scale_up(), health: Health.passing()) | ||
|
|
||
| %Trento.HostReadModel{id: host_1_id} = |
There was a problem hiding this comment.
We could rename host_1_id to host_id
| database_health: :warning, | ||
| clusters_health: :passing, | ||
| hosts_health: :unknown, | ||
| database_id: ^sap_system_id, |
There was a problem hiding this comment.
now I bit confused it this is the database id or the sap system id hehe
Because if it is the same, we are sending it twice, in the id and in the database_id.
Now i'm thinking both of them might have the same id, the difference in the frontend is that we get one with /sap_systems/id and the other with /databases/id.
Could you double check this?
There was a problem hiding this comment.
As far as I understood, they are not the same 👀
There was a problem hiding this comment.
They can but can also be different
Description
The health overview api endpoint, has two new fields, the
database_idandhana_cluster_id, we need them in order to achieve new functionalities in the frontend that consumes this api endpoint.How was this tested?
Integration testing for both service and endpoint.