Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

bug: ValueError: Out of range float values are not JSON compliant #2213

Open
1 task
dgawlowsky opened this issue Feb 3, 2024 · 2 comments
Open
1 task
Labels
kind/Bug Something isn't working valuestream/SDK

Comments

@dgawlowsky
Copy link
Contributor

Singer SDK Version

0.34.1

Is this a regression?

  • Yes

Python Version

3.9

Bug scope

Taps (catalog, state, etc.)

Operating System

MacOS

Description

I get ValueError: Out of range float values are not JSON compliant when trying to sync Amazon Document DB collection.

The probably reason is that simple_json.dumps() has allow_nan=False default, while standard dumps() function from standard json library has allow_nan=True.

As there is no way to pass configuration from outside could we add this functionality? I can introduce relevant fix.

Code

No response

@dgawlowsky dgawlowsky added kind/Bug Something isn't working valuestream/SDK labels Feb 3, 2024
@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Feb 3, 2024

Hi @dgawlowsky, thanks for logging!

I think having allow_nan by default would violate the interoperability principle in the Singer ecosystem since it's specific to Python's JSON implementation: while most taps and targets are written in Python, they could be programmed in any language.

There's also the question of how the jsonchema library that we use for validation on the target side would treat the NaN values, although it's now possible to disable validation on the user end (631d5df), if they're fine with moving NaNs around and skipping schema validation.

Do start a PR and we can discuss the details there!

Related: #2211

@dgawlowsky
Copy link
Contributor Author

Hi @edgarrmondragon - thanks for prompt response again. Opened PR with this one line change. Let's discuss there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/Bug Something isn't working valuestream/SDK
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants