CM-1099 LiveArt External Adapter #4018
Conversation
…d for preparing requests and parsing response
🦋 Changeset detectedLatest commit: 26d82b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
…js into feature/CM-1099_liveart_ea_nav
…matted frse files by mistake
| */ | ||
| export function parseResponse( | ||
| params: TypeFromDefinition<typeof inputParameters.definition>[], | ||
| response: AxiosResponse<ResponseSchema>, |
There was a problem hiding this comment.
Can this reference to Axios be removed?
Doesn't look like it's being used anywhere (other than a passed in type) and it's adding a bunch of new packages and preventing the checks from succeeding.
| description: 'The API URL for the LiveArt data provider', | ||
| type: 'string', | ||
| required: true, | ||
| default: 'https://artwork-price-oracle-api-dev-ms.liveart.ai', |
There was a problem hiding this comment.
this is their prod API?
| total_shares: number | null | ||
| nav_per_share: string | null | ||
| valuation_price_date: string | null | ||
| valuation_price: string | null |
There was a problem hiding this comment.
Will these be returned as explicit null or just missing from the API response ie: undefined? If just missing you can try replacing the nulls by ?:
…ractkit/external-adapters-js into feature/CM-1099_liveart_ea_nav
…js into feature/CM-1099_liveart_ea_nav
| @@ -0,0 +1,34 @@ | |||
| { | |||
| "name": "@chainlink/liveart-external-adapter", | |||
There was a problem hiding this comment.
[nit] I don't think any of our other adapters are named 'xxx-external-adapter', just 'xxx-adapter', also this will have effects on the changeset name
| }, | ||
| "license": "MIT", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1", |
There was a problem hiding this comment.
usually there is no point to update this file from the generator output.
There was a problem hiding this comment.
i could not generate this via generator due to weird interaction with pnpm on my system ... I had to create it manually
There was a problem hiding this comment.
will this ever be used to populate data on a feed? If not, this endpoint and transport isn't needed, remove
There was a problem hiding this comment.
its on the jira ticket, i have no idea, need to ask product
| import { assets } from './endpoint/assets' | ||
|
|
||
| export const adapter = new Adapter({ | ||
| defaultEndpoint: assets.name, |
There was a problem hiding this comment.
this will need a change it you remove the assets endpoint
There was a problem hiding this comment.
consider calling this endpoint nav (update both the filename and the name field in the AdapterEndpoint) to be in line with other nav EAs.
There was a problem hiding this comment.
but its not a nav? ticket says to return all the data from the endpoint?
| Parameters: typeof inputParameters.definition | ||
| Response: { | ||
| Data: Asset | ||
| Result: null |
There was a problem hiding this comment.
core node reads either a top level result field or data.result. Usually best practice to write the data point you want to go on chain here at the top level.
| params: param, | ||
| response: { | ||
| errorMessage: `Mismatched asset_id in response. Expected ${param.asset_id}, got ${response.data.asset_id}`, | ||
| statusCode: 500, |
There was a problem hiding this comment.
maybe 502 to follow the pattern we use in other EAs
| return { | ||
| params: param, | ||
| response: { | ||
| result: null, |
There was a problem hiding this comment.
update here once you update the type please
| }) | ||
| }, | ||
| parseResponse: (params, response) => { | ||
| return params.map((param: TypeFromDefinition<typeof inputParameters.definition>) => { |
There was a problem hiding this comment.
do you need to specify the type here?
There was a problem hiding this comment.
i'm guessing this can be removed
There was a problem hiding this comment.
if only used in one location you can simplify and stick this interfact in that file.
| @@ -0,0 +1,57 @@ | |||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | |||
|
|
|||
| exports[`LiveArt NAV endpoints /asset/\${asset_id} should handle upstream bad response 1`] = ` | |||
There was a problem hiding this comment.
this variable in the test title looks wrong, similar below
| linkType: soft | ||
|
|
||
| "@chainlink/liveart-external-adapter@workspace:packages/sources/liveart": | ||
| version: 0.0.0-use.local |
There was a problem hiding this comment.
please rerun yarn && yarn setup once you update your package.json file with the name change.
3 participants
Closes OPDATA-4029 / CM-1099
Description
LiveArt External Adapter (EA) to fetch art nav_per_share numerical data.
......
Changes
Steps to Test
From root directory
Quality Assurance
infra-k8sconfiguration file.adapter-secretsconfiguration file or update the soak testing blacklist.test-payload.jsonfile with relevant requests.feature/x,chore/x,release/x,hotfix/x,fix/x) or is created from Jira.