Fix for decimal json serialization error

XiaozhouWang85 commented 1 year ago

Problem

Describe the problem your PR is trying to solve

I'm using Meltano to pull data from Postgres and write to S3. I'm using the Pipelinewise variant in both cases and both are latest version (2.0.0).

When running:

meltano run tap-postgres target-s3-csv

I get the following error:

TypeError: Object of type 'Decimal' is not JSON serializable

Seems like others have been facing this issue too:

https://meltano.slack.com/archives/C01TCRBBJD7/p1637661633250900 https://meltano.slack.com/archives/CFG3C3C66/p1610696969011800

I've traced the error to this line. At some point (maybe in tap-postgres, the numbers are converted into Python Decimal type which cannot be handled by the standard Python JSON library.

Proposed changes

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue. Swapping for simplejson fixes the issue in my runs. I haven't updated setup.py as simplejson is already installed as part of other dependencies.

Types of changes

What types of changes does your code introduce to PipelineWise? Put an x in the boxes that apply

[x] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] Documentation Update (if none of the other choices apply)

Checklist

[ ] Description above provides context of the change
[ ] I have added tests that prove my fix is effective or that my feature works
[ ] Unit tests for changes (not needed for documentation changes)
[ ] CI checks pass with my changes
[ ] Bumping version in setup.py is an individual PR and not mixed with feature or bugfix PRs
[ ] Commit message/PR title starts with [AP-NNNN] (if applicable. AP-NNNN = JIRA ID)
[ ] Branch name starts with AP-NNN (if applicable. AP-NNN = JIRA ID)
[ ] Commits follow "How to write a good git commit message"
[ ] Relevant documentation is updated including usage instructions

XiaozhouWang85 commented 1 year ago

Hi @Samira-El, I'm bumping into this issue with my pipeline and this 1 line changes fixes things for me. Would a maintainer be able to take a look and merge it to the main repo if it's appropriate.

XiaozhouWang85 commented 1 year ago

Hi @koszti would you be able to have a quick look at this?

dluftspring commented 1 year ago

Also running into this error and would be great to get this merged! Can just point my Meltano project at the fix branch in the interim thanks @XiaozhouWang85

mghastin commented 1 year ago

^+1 I am also having this issue. Thanks @XiaozhouWang85

pnadolny13 commented 1 year ago

@Samira-El would you have time to review this PR? It looks pretty straight forward and has been popping up quite a bit in the Meltano slack channel recently.

If anyone is looking for a short term solution until this is merged, like @dluftspring said in https://github.com/transferwise/pipelinewise-target-s3-csv/pull/171#issuecomment-1368242883 you can update your meltano.yml config to something like:

  - name: target-s3-csv
    variant: transferwise
    pip_url: git+https://github.com/XiaozhouWang85/pipelinewise-target-s3-csv.git@fix-for-Decimal-JSON-serialization-error

Then run meltano install loader target-s3-csv --clean to reinstall the package using the new pip url.

transferwise / pipelinewise-target-s3-csv