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

JSON InputStreamReader #259

Closed
wants to merge 2 commits into from
Closed

Conversation

cgardens
Copy link
Contributor

@cgardens cgardens commented Sep 14, 2020

What

  • Currently we read from a singer tap by using lines(). If a json object is split across multiple lines, we will not be able to parse it.

How

  • Use jackson parser to parse the InputStream. It handles this cross line issue. One downside is that it throws a lot of exceptions for any non json in the InputStream (on per word). My inclination is to use this implementation instead of trying to go lower level because I think doing so will be more prone to error. Curious to hear other people's opinions on this. It's the jackson json parser that throws all of these errors. If we want to use the jackson parser then we're going to have to deal with it.

@michel-tricot - this is branch is off of your branch, but I did not hook it up since you're still changing things.

@jrhizor
Copy link
Contributor

jrhizor commented Sep 14, 2020

@michel-tricot

Since the Singer spec has single line messages as part of the contract ("A Tap outputs structured messages to stdout in JSON format, one message per line."), why is this necessary? Wouldn't a tap producing multi-line messages be broken?

@michel-tricot
Copy link
Contributor

Can we try on a postgres database and see how a blog of text with \n would behave?

@michel-tricot
Copy link
Contributor

got closed because I removed the branch you depended on.

But based on Jared's finding, let's hold-off on this

davydov-d added a commit that referenced this pull request Jun 8, 2022
davydov-d added a commit that referenced this pull request Jun 8, 2022
davydov-d added a commit that referenced this pull request Jun 8, 2022
davydov-d added a commit that referenced this pull request Jun 9, 2022
davydov-d added a commit that referenced this pull request Jun 9, 2022
davydov-d added a commit that referenced this pull request Jun 9, 2022
* #259 on call: Source Hubspot - fix for the property_history stream which did not emit any records

* #259 Source Hubspot: upd changelog

* #259 oncall: hubspot review fixes

* auto-bump connector version

Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
@swyxio swyxio deleted the cgardens/json_stream_reader branch October 12, 2022 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants