singer-io / singer-python

Writes the Singer format from Python
https://singer.io
Apache License 2.0
544 stars 129 forks source link

Transformer dumps JSON incompatible string #136

Open daigotanaka opened 3 years ago

daigotanaka commented 3 years ago

When Transformer recognizes the type to be str, it will convert the (sub) object to str type. The issue is, if such (sub) object's original type is dict, the current method of converting to str produces JSON incompatible string: https://github.com/singer-io/singer-python/blob/6472683c7b46b9caa0969c5d44a62db07b811b30/singer/transform.py#L288

This results in the conversion from a dict

{'active': True, 'note': None}

to

"{'active': True, 'note': None}"

instead of

'{"active": true, "note": null}'

str(data) conversion seems to produce problems with escape characters as well.

I am wondering if it is acceptable to replace str(data) with json.dumps(data)

One may argue that the tap should fully specify the schema so that the (sub)object is written out as dict. However, many of the Rest API often includes the field whose schema is not static. An example is Github API's event object. event.payload is a (dict) object, but the schema depends on the event type. https://docs.github.com/en/free-pro-team@latest/developers/webhooks-and-events/github-event-types#event-object-common-properties In fact, I discovered this issue while I was debugging tap-github's usage of Transformer: https://github.com/singer-io/tap-github/blob/master/tap_github/__init__.py#L361

If the str conversion was done through json.dumps, it would have been possible to parse JSON in the target datastore such as BigQuery and Redshift.