Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat(taps): Numeric values are now parsed as decimal.Decimal in REST and GraphQL stream responses #2780

Merged
merged 1 commit into from
Nov 28, 2024

Conversation

edgarrmondragon
Copy link
Collaborator

@edgarrmondragon edgarrmondragon commented Nov 28, 2024

This means under normal circumstances, decimal.Decimal would be used across the board:

  1. A float value of "value": 3.14 in the API response is parsed as Decimal("3.14")
  2. The Decimal("3.14") value is serialized to JSON as value: 3.14 in the tap output
  3. The float value of "value": 3.14 in the RECORD Singer message will be parsed as Decimal("3.14") by the target.

That should ensure we preserve precision through the pipeline.

Caution

Now, this could be a breaking change if folks are doing floating-point math with these values, but I think it's rare enough that we can make do with a call-out in the release notes.


📚 Documentation preview 📚: https://meltano-sdk--2780.org.readthedocs.build/en/2780/

@edgarrmondragon edgarrmondragon changed the title feat(taps): Numeric values are now deserialized as decimal.Decimal in REST and GraphQL streams feat(taps): Numeric values are now parsed as decimal.Decimal in REST and GraphQL stream responses Nov 28, 2024
Copy link

codspeed-hq bot commented Nov 28, 2024

CodSpeed Performance Report

Merging #2780 will not alter performance

Comparing edgarrmondragon/feat/rest-number-decimal (a92b8a7) with main (8474a24)

Summary

✅ 6 untouched benchmarks

Copy link

codecov bot commented Nov 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.88%. Comparing base (8474a24) to head (a92b8a7).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2780   +/-   ##
=======================================
  Coverage   90.88%   90.88%           
=======================================
  Files          62       62           
  Lines        5165     5166    +1     
  Branches      667      667           
=======================================
+ Hits         4694     4695    +1     
  Misses        330      330           
  Partials      141      141           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edgarrmondragon edgarrmondragon self-assigned this Nov 28, 2024
@edgarrmondragon edgarrmondragon added the Release Highlight Call this out in the release notes label Nov 28, 2024
@edgarrmondragon edgarrmondragon added this to the v0.43 milestone Nov 28, 2024
@edgarrmondragon edgarrmondragon merged commit 3d8e418 into main Nov 28, 2024
38 checks passed
@edgarrmondragon edgarrmondragon deleted the edgarrmondragon/feat/rest-number-decimal branch November 28, 2024 19:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Release Highlight Call this out in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant