2702/expose new btp destination creation #2799
Conversation
…new-btp-destination-creation
🦋 Changeset detectedLatest commit: 2540d98 The changes in this PR will be included in the next version bump. This PR includes changesets to release 35 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Change has been merged into bas-sdk, waiting on release https://www.npmjs.com/package/@sap/bas-sdk?activeTab=versions v3.11.2 or higher to consume the changes. |
longieirl
force-pushed
the
2702/expose-new-btp-destination-creation
branch
from
ee6a8be to
86dd1b2
Compare
|
| * @param logger Logger | ||
| * @returns name representing the newly created SAP BTP destination | ||
| */ | ||
| export async function createOAuth2UserTokenExchangeDest( |
There was a problem hiding this comment.
What is the result for a consumer of this API when something fails? Should you throw or document what is returned in this case please?
| serviceInstanceName, | ||
| logger | ||
| ); | ||
| await destinations.createDestination(basSDKDestination); |
There was a problem hiding this comment.
Can you instead return the new destination? This call appears to throw so it would be good to catch and doc what the errors mean so consumers can use this to code against and not have to dig into the underlying APIs...
|
|
||
| if (!destinationInstance) { | ||
| // Create a new abap-cloud destination instance on the target CF subaccount | ||
| await apiCreateServiceInstance('destination', 'lite', DESTINATION_INSTANCE_NAME, null); |
There was a problem hiding this comment.
Is the destination not returned here?
| ); | ||
| await destinations.createDestination(basSDKDestination); | ||
| logger?.debug(`SAP BTP destination ${JSON.stringify(basSDKDestination, null, 2)} created.`); | ||
| return basSDKDestination.name; |
There was a problem hiding this comment.
Should you not return the whole Destination object for better usability and consumer wont need to make more API calls subsequently to fetch the destination?
| DestinationType | ||
| } from './destination'; | ||
| import type { ServiceInfo } from './service-info'; | ||
| import { destinations } from '@sap/bas-sdk'; |
There was a problem hiding this comment.
Disambiguation :
| import { destinations } from '@sap/bas-sdk'; | |
| import { destinations as destinationAPI } from '@sap/bas-sdk'; |
Fix for #2702
Requires a newer version of bas-sdk to be published for the token exchange flow to work.