Feat/OPDATA-6509 support data engine v7 schema & returnAs: float#4770
Feat/OPDATA-6509 support data engine v7 schema & returnAs: float#4770mmcallister-cll wants to merge 4 commits intomainfrom
Conversation
🦋 Changeset detectedLatest commit: ae9868b The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 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 |
|
👋 mmcallister-cll, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
mohamed-mehany
left a comment
There was a problem hiding this comment.
Looks good, minor comments about edge cases
| if (returnAs === 'float') { | ||
| // scaled is the actual value, return as decimal string | ||
| return scaled.toString() | ||
| } |
There was a problem hiding this comment.
When fromDecimals === toDecimals, the function returns immediately, bypassing the 'float' logic entirely. If someone passes decimals: 18 (same as native) with returnAs: 'float', they'd get the raw integer "1156789000000000000" instead of "1.156789".
There was a problem hiding this comment.
That looks right to me 🤔. The intent is to perform the same scale-to num_decimals operation as already exists (scale to a certain # of decimals *10^X), but preserve the floating point precision rather than truncating. Am I missing something?
There was a problem hiding this comment.
Ah I see, I thought "returnAs" meant always return the value as float-point decimal, but I guess it means to return as float only when scaling is required, as an alternative to truncation? maybe worth adding a unit test or 2 to explicitly document the expected behavior.
| type: 'number', | ||
| description: 'Number of decimals to scale the resultPath value to (from native 18)', | ||
| }, | ||
| returnAs: { |
There was a problem hiding this comment.
float vs int
Use case is using in a multiply with implied-price. Prob most correct is to add multiply/divide by constant to implied-price, and then nest implied-price calling itself, but figured we already have scaling functionality here.
There was a problem hiding this comment.
Yeah, I think we should update implied-price to multiply two numbers with decimals instead
…ic notation for low/high values
Closes #OPDATA-6509
Description
Adds support for data-engine v7 report schema
Adds functionality to decimal scaling via
returnAs: 'float'input param. Previous behaviour was truncate-only. Truncate is preserved as default behaviour for backward compatibility.Changes
exchangeRate-v7(redemptionRate-v7) endpointreturnAscommon inputParam to allow for scaling selection (+ required downstream code changes)Steps to Test
yarn test data-engine{ "data": { "endpoint": "redemptionRate-v7", "feedId": "0x0007621b6f5eb0b5bd5a8bb3d52902b3e19498ba45781e5ddf92ce7e03d7ecef", "resultPath": "exchangeRate", "decimals": 0, "returnAs": "float" } }Quality Assurance
infra-k8sconfiguration file.adapter-secretsconfiguration file.test-payload.jsonfile with relevant requests.feature/x,chore/x,release/x,hotfix/x,fix/x) or is created from Jira.