Fix grafana component to add and remove connections. #13199
Conversation
Signed-off-by: Pratham <[email protected]>
This PR fixes meshery#12958 Signed-off-by: Pratham <[email protected]>
|
|
shanukun
temporarily deployed
to
staging-playground
GitHub Actions
Inactive
END-TO-END TESTS
📦 Test Result Summary
⌛ Duration: 7 minutes and 29 seconds Overall Result: 👎 Some tests failed. [Show/Hide] Test Result Details
|
|
I don't know why this would fail. |
| return nil, http.StatusInternalServerError, ErrUnmarshal(err, "connection") | ||
| } | ||
| return &conn, resp.StatusCode, nil | ||
| return connectionPage.Connections[0], resp.StatusCode, nil |
There was a problem hiding this comment.
@shanukun ideally, we don't use numeric indexing, unless there's only ever one element in the array.
There was a problem hiding this comment.
It is in the method for finding a connection by ID, and should not yield more than one connection. We can additionally check to ensure that there is at least one connection in the list.
There was a problem hiding this comment.
@shanukun have you tried using a active grafana connection?
Are you able to successfully transition the status of connection and able to view any charts?
| }; | ||
|
|
||
| if (typeof data?.credential_id !== 'undefined') { | ||
| getCredentialByID(data?.credential_id).then((res) => { |
There was a problem hiding this comment.
@shanukun this seems a concern here. Can you verify that?
The ui currently expecting to get the details of api key from the secret value of credentials api but currently we don't have that way to add that secret so you would probably get this value as undefined.
There was a problem hiding this comment.
Users are to supply the API key, if their Grafana needs it.
There was a problem hiding this comment.
When creating a new connection, users provide the API key. However, if a connection has already been established, the only option to acquire the API key is to use the getCredentialByID function. The credential_id may be obtained from a grafana config request (/api/telemetry/metrics/grafana/config).
However, after looking into the Board Search textfield and handleChange closely, I think this function needs to be called right after retrieving the connection config.
There was a problem hiding this comment.
@shanukun, will you confirm that which Connection Definition (from which Model) is being used here?
I’m using a local grafana server. I’m able to transition the status. The Board Search textfield, GrafanaCustomChart component, Add Panel action to some extent, and the endpoint for saving selected boards is still broken, so even if I can see the panels, the charts shouldn't appear yet. I'm working on this, but I'm not sure if I should create a separate PR for these changes. |
The one in |
|
@shanukun |
Notes for Reviewers
Signed commits