-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(alpha): simulator integration API #1035
Conversation
public deleteIntegrations = async (ids: IdEither[]) => { | ||
return this.integrationsApi.delete(ids); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't think we have delete integrations (yet), are you suggesting on making it future proof? :)
packages/beta/src/types.ts
Outdated
export interface SimulatorResourceChange | ||
extends SimulatorResourcePatch, | ||
InternalId { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export interface SimulatorResourceChange | |
extends SimulatorResourcePatch, | |
InternalId { } | |
export interface SimulatorChangeById extends SimulatorPatch, InternalId {} | |
export interface SimulatorChange = SimulatorChangeById; |
Let's make it two step, in case we need to extend with SimulatorChangeByExternalId (easily)
packages/beta/src/cogniteClient.ts
Outdated
@@ -55,6 +61,7 @@ export default class CogniteClient extends CogniteClientCleaned { | |||
|
|||
this.httpClient.setDefaultHeader('cdf-version', 'beta'); | |||
|
|||
this.simulatorsApi = this.apiFactory(SimulatorsAPI, 'simulators'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this API beta?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, it's alpha, we'll move it to alpha for now.
…dk-js into onboard-simint-api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, need to merge this so we can iterate further
No description provided.