TS migrations #221

ebarakos · 2021-01-12T15:47:18Z

Migrating the following adapters to Typescript:

google-finance
kaiko
marketstack
metalsapi
nikkei
nomics
openexchangerates
poa-gasprice
polygon

github-actions · 2021-01-12T15:48:19Z

I see that you haven't updated any CHANGELOG files. Would it make sense to do so?

justinkaseman

Great!

Added a few small tweaks.

One thing I am unsure of is what the endpoint would be for some of these adapters. I know we want to keep similar endpoints conformed to the same name and params. Just thinking aloud that we need them documented somewhere.

justinkaseman · 2021-01-14T21:17:03Z

kaiko/src/index.ts

+import { makeExecute } from './adapter'
+import { makeConfig } from './config'
+
+const NAME = 'EXAMPLE'


Suggested change

const NAME = 'EXAMPLE'

const NAME = 'KAIKO'

@krebernisak Do we want this capitalized or lowercase? I've seen both ways, but let's pick a convention.

This was first used as an ENV prefix to avoid naming collisions when using two or more adapters in the same env. So for now it was a simple hardcoded uppercase string

What we should do now to make this more robust is this:

Import the name from package.json, remove the -adapter suffix, and export that as NAME.

Create a toEnvName util function that will map it to a safe string to use as ENV name

Environment variable names used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase letters, digits, and the '_' (underscore) from the characters defined in Portable Character Set and do not begin with a digit.

justinkaseman · 2021-01-14T21:18:11Z

kaiko/src/config.ts

+import { Requester } from '@chainlink/external-adapter'
+import { Config } from '@chainlink/types'
+
+export const DEFAULT_ENDPOINT = 'example'


This is currently an unused variable as there is only one endpoint

justinkaseman · 2021-01-14T21:24:50Z

marketstack/package.json

+    "test": "mocha --exit --timeout 0 -r ts-node/register 'test/**/*.test.ts'",
+    "test:unit": "mocha --exit --grep @integration --invert -r ts-node/register 'test/**/*.test.ts'",
+    "test:integration": "mocha --exit --timeout 0 --grep @integration -r ts-node/register 'test/**/*.test.ts'",
+    "server": "node -e 'require(\"./index.js\").server()'",


Suggested change

"server": "node -e 'require(\"./index.js\").server()'",

Take these out of every TS migrated EA as ./index.js is now gone.

Taking it out of the example as well.

justinkaseman · 2021-01-14T21:27:23Z

marketstack/src/adapter.ts

+    case eod.Name: {
+      const data = await eod.execute(config, request)
+
+      return Requester.success(jobRunID, {


This will quickly grow to be hard to read if say 10 other endpoints were added.

I think the best style would be how Jonas modified the example adapter as seen here:
https://github.com/smartcontractkit/external-adapters-js/blob/develop/example/src/endpoint/example.ts#L32
Each endpoint handles its own return, which keeps adapter.ts a lot cleaner.

justinkaseman · 2021-01-14T21:30:02Z

marketstack/src/endpoint/eod.ts

+
+export const Name = 'eod'
+
+const customError = (data: any) => data.Response === 'Error'


@krebernisak Should we be adding typing for each API response shape? Seems like it might be hard to maintain, but could provide valuable debugging information.

Not in this PR to keep it fast & simple.

But in the future, we should type anything that's useful, and I think that will become useful at one point. The further we go with composite adapters, the more we use the data provider adapters programmatically, the more useful we are going to find those types.

First, we should finish this migration, then focus on improvements to our internal framework data types, and then we can make another push like this where we add a bunch of underlying API data types in one push. At that point it could start being useful :)

marketstack/src/index.ts

justinkaseman · 2021-01-14T21:34:10Z

metalsapi/package.json

@@ -1,15 +1,46 @@
 {
  "name": "@chainlink/metalsapi-adapter",
  "version": "0.0.1",


Should we be bumping versions for all of these TS migrations?

Yeah, I think we should!

Since all versions are now set to 0.0.1, let's wait until everything is finished/merged, and batch-update all TS migrated ones, or all TS ones in general.

metalsapi/src/index.ts

nikkei/src/index.ts

nomics/src/index.ts

ebarakos · 2021-01-27T15:26:07Z

Converted this to draft as it had been split to 3 smaller batches. To be closed after merging the migrations.

ebarakos force-pushed the feature/176014837-TS-migration branch 3 times, most recently from 70a3152 to b3fc300 Compare January 14, 2021 15:14

justinkaseman requested changes Jan 14, 2021

View reviewed changes

ebarakos force-pushed the feature/176014837-TS-migration branch from b3fc300 to 7236e27 Compare January 18, 2021 09:50

krebernisak mentioned this pull request Jan 18, 2021

Adapters should automatically export their name #244

Closed

ebarakos linked an issue Jan 18, 2021 that may be closed by this pull request

Add API's response.data on the data field of adapter's response #235

Closed

ebarakos force-pushed the feature/176014837-TS-migration branch 5 times, most recently from 945a5f9 to 319ea92 Compare January 22, 2021 14:46

ebarakos added 17 commits January 25, 2021 17:10

google-finance TS migration

0c1484f

kaiko TS migration

b27a65b

marketstack TS migration

84371a4

metalsapi TS migration

cfd4d5c

nikkei TS migration

9a8f1dc

nomics TS migration

fb5f5c8

Added Config type

133c310

Missing type def

d38fc89

openexchangerates TS migration

69dd209

poa-gasprice TS migration

541c230

polygon TS migration

8ddef65

missing files and fixes

41b6e4e

reverted tradingeconomics

ba2d56a

added CHANGELOG

9cfb463

rebased + review changes

47cbb2c

fix unused config

9d317b8

add API's response data to adapter's response

159c760

ebarakos force-pushed the feature/176014837-TS-migration branch from 319ea92 to 159c760 Compare January 25, 2021 15:10

ebarakos marked this pull request as draft January 27, 2021 15:25

ebarakos closed this Feb 3, 2021

austinborn deleted the feature/176014837-TS-migration branch November 4, 2021 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TS migrations #221

TS migrations #221

ebarakos commented Jan 12, 2021 •

edited

Loading

github-actions bot commented Jan 12, 2021

justinkaseman left a comment

justinkaseman Jan 14, 2021

krebernisak Jan 15, 2021

justinkaseman Jan 14, 2021 •

edited

Loading

justinkaseman Jan 14, 2021

ebarakos Jan 18, 2021

justinkaseman Jan 14, 2021

justinkaseman Jan 14, 2021

krebernisak Jan 15, 2021

justinkaseman Jan 14, 2021

krebernisak Jan 15, 2021

ebarakos Jan 18, 2021

ebarakos commented Jan 27, 2021


		export const Name = 'eod'

		const customError = (data: any) => data.Response === 'Error'

TS migrations #221

TS migrations #221

Conversation

ebarakos commented Jan 12, 2021 • edited Loading

github-actions bot commented Jan 12, 2021

justinkaseman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinkaseman Jan 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ebarakos commented Jan 27, 2021

ebarakos commented Jan 12, 2021 •

edited

Loading

justinkaseman Jan 14, 2021 •

edited

Loading