Skip to content

[DF-22956] ensure tiingo request transform executes by removing factory wrapper#4778

Open
guandali wants to merge 8 commits intomainfrom
lli/tiingo-fix
Open

[DF-22956] ensure tiingo request transform executes by removing factory wrapper#4778
guandali wants to merge 8 commits intomainfrom
lli/tiingo-fix

Conversation

@guandali
Copy link
Copy Markdown
Member

@guandali guandali commented Mar 26, 2026

Closes #DF-22956

Description

Refactor tiingoCommonSubscriptionRequestTransform from a factory function to a direct request transform to align with the AdapterEndpoint contract.

Changes

Before

export function tiingoCommonSubscriptionRequestTransform() {
  return (req) => { ... }
}

After

export const tiingoCommonSubscriptionRequestTransform = (req) => { ... }

Steps to Test

  1. Steps
  2. to
  3. test

Quality Assurance

  • If a new adapter was made, or an existing one was modified so that its environment variables have changed, update the relevant infra-k8s configuration file.
  • If a new adapter was made, or an existing one was modified so that its environment variables have changed, update the relevant adapter-secrets configuration file.
  • If a new adapter was made, or a new endpoint was added, update the test-payload.json file with relevant requests.
  • The branch naming follows git flow (feature/x, chore/x, release/x, hotfix/x, fix/x) or is created from Jira.
  • This is related to a maximum of one Jira story or GitHub issue.
  • Types are safe (avoid TypeScript/TSLint features like any and disable, instead use more specific types).
  • All code changes have 100% unit and integration test coverage. If testing is not applicable or too difficult to justify doing, the reasoning should be documented explicitly in the PR.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 26, 2026

🦋 Changeset detected

Latest commit: b0e9980

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@chainlink/por-address-list-adapter Patch
@chainlink/tiingo-adapter Patch
@chainlink/itick-adapter Patch
@chainlink/token-allocation-adapter Patch
@chainlink/bsol-price-adapter Patch
@chainlink/crypto-volatility-index-adapter Patch
@chainlink/savax-price-adapter Patch
@chainlink/set-token-index-adapter Patch
@chainlink/xsushi-price-adapter Patch

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

Copy link
Copy Markdown
Contributor

@cl-efornaciari cl-efornaciari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one question, also needs to be rebased

})

it('lowercases base-only requests without a quote', () => {
const requestContextData = { base: 'AaPl' }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an actual case? is this assumed to be USD?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

USD

@@ -0,0 +1,5 @@
---
'@chainlink/tiingo-adapter': major
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this update tiingo-state as well?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For major bump always include details on that what is breaking and how should NOP handle it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i downgraded it patch
changeset won't list tiingo-state as option, i think in this PR it is implicitly modified

Comment on lines +2 to +4
'@chainlink/por-address-list-adapter': patch
'@chainlink/tiingo-adapter': patch
'@chainlink/itick-adapter': patch
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes no sense, you only updated tiingo

Comment on lines +16 to +17
expect(req.requestContext.data).toBe(requestContextData)
expect(req.requestContext.data).toEqual({ base: 'eth', quote: 'usd' })
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what toBe is doing here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants