[OCTRL-1012]: GetEnvironments is missing ODC devices info even with flag set#720
[OCTRL-1012]: GetEnvironments is missing ODC devices info even with flag set#720
Conversation
core/protos/o2control.proto
Outdated
| message GetEnvironmentsRequest { | ||
| bool showAll = 1; | ||
| bool showTaskInfos = 2; | ||
| bool showIntegratedServices = 3; |
There was a problem hiding this comment.
I think the flag name is unclear. Even when it's false, we fill ingratedServicesData with the output of GetEnvironmentsShortData, while the flag name gives impression we will get nothing. Could we name it for example showDetailedIntegratedServices, or anything conveying this meaning? A short comment might also help.
The fact that showAll does not imply we show task infos is another issue, but it's something that was there before.
core/server.go
Outdated
|
|
||
| if request.GetShowIntegratedServices() { | ||
| e.IntegratedServicesData = integration.PluginsInstance().GetEnvironmentsData([]uid.ID{env.Id()})[env.Id()] | ||
| } | ||
|
|
There was a problem hiding this comment.
Is there a reason to put this code here instead of combining it with this call from above?
integratedServicesEnvsData := integration.PluginsInstance().GetEnvironmentsShortData(m.state.environments.Ids())
Depending on the flag, we could call either GetEnvironmentsShortData or GetEnvironmentsData instead of calling always the first, and optionally overwriting it with the 2nd.
core/server.go
Outdated
| integratedServicesEnvsData = integration.PluginsInstance().GetEnvironmentsShortData(m.state.environments.Ids()) | ||
| } | ||
|
|
||
| // Get plugin-provided environment data for all envs |
There was a problem hiding this comment.
this comment talks about what is done above. can you move it to the top of the corresponding code snippet?
…ntegrated plugins
2 participants
No description provided.