Prepare the connector GetAll API for versioning #162799
Conversation
ersin-erdal
added
release_note:skip
|
Pinging @elastic/response-ops (Team:ResponseOps) |
There was a problem hiding this comment.
Should we use this to validate getAll API response?
| id: schema.string(), | ||
| name: schema.string(), | ||
| actionTypeId: schema.string(), | ||
| config: schema.maybe(schema.recordOf(schema.string(), schema.any())), |
There was a problem hiding this comment.
Should we get the config schema from the connectorType (validate.config.schema) and use here?
There was a problem hiding this comment.
It would be worth exploring as a follow up. We may need a fn like getConnectorSchema where it returns this schema with the config and secrets having proper validation.
# Conflicts: # x-pack/plugins/actions/server/actions_client/actions_client.ts
| isSystemAction, | ||
| ...res | ||
| }) => ({ | ||
| ...res, |
There was a problem hiding this comment.
Ideally, we should avoid spreading for transforms, especially for versioned transforms to avoid properties accidentally getting into response types. I think it would be better if we were more specific about the properties.
|
|
||
| export const transformGetAllConnectorsResponse: RewriteResponseCase<FindConnectorResult[]> = ( | ||
| results | ||
| ): ConnectorResponse[] => { |
There was a problem hiding this comment.
Let's use the versioned type here since this is also a versioned file (ConnectorResponseV1). It let's us know to increment this file when the underlying schema has changed
| import { ConnectorResponse } from '../../../../../../common/routes/connector/response'; | ||
| import { RewriteResponseCase } from '../../../../../../common/rewrite_request_case'; | ||
|
|
||
| export const transformGetAllConnectorsResponse: RewriteResponseCase<FindConnectorResult[]> = ( |
There was a problem hiding this comment.
Instead of using RewriteResponseCase, I think we should be explicit with the type transforms. I address this in a later comment.
| const actionsClient = (await context.actions).getActionsClient(); | ||
| const result = await actionsClient.getAll(); | ||
| return res.ok({ | ||
| body: transformGetAllConnectorsResponse(result), |
There was a problem hiding this comment.
Let's use the V1 version of the transform, it will let us know when the response schema has changed.
Also I like to version the body explicitly:
const responseBody: ConnectorResponseV1 = transformGetAllConnectorsResponse(result)
There was a problem hiding this comment.
Thanks for reviewing Jiawei. I addressed all the comments.
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Public APIs missing exports
History
To update your PR or re-run it, just comment with: |
| id: schema.string(), | ||
| name: schema.string(), | ||
| actionTypeId: schema.string(), | ||
| config: schema.maybe(schema.recordOf(schema.string(), schema.any())), |
There was a problem hiding this comment.
It would be worth exploring as a follow up. We may need a fn like getConnectorSchema where it returns this schema with the config and secrets having proper validation.
Part of: https://github.com/elastic/response-ops-team/issues/125
This PR intends to prepare the
GET ${BASE_ACTION_API_PATH}/connectorsAPI for versioning as shown in the above issue.