Skip to content

enhancement(http sink): Add JSON encoding option#1174

Merged
binarylogic merged 5 commits intovectordotdev:masterfrom
jamessewell:http-json-input
Nov 25, 2019
Merged

enhancement(http sink): Add JSON encoding option#1174
binarylogic merged 5 commits intovectordotdev:masterfrom
jamessewell:http-json-input

Conversation

@jamessewell
Copy link
Copy Markdown
Contributor

Basically just the same as NDJSON, except you delimit with , (not \n) and then
remove the final , and wrap in an array.

Basically just the same as NDJSON, except you delimit with , and then
wrap in an array.
@binarylogic
Copy link
Copy Markdown
Contributor

Thanks @jamessewell! @lukesteensen, since @LucioFranco is out I wouldn't mind you taking a quick look at this.

Copy link
Copy Markdown
Member

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

So that the docs stay up to date, could you add the new encoding type to the list here?

Encoding::Ndjson => builder.header("Content-Type", "application/x-ndjson"),
Encoding::Json => {
body.insert(0, b'[');
body.pop();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the trailing comma from the last record we need to remove? Might be worth a little comment.

@binarylogic binarylogic removed the needs: docs Needs documentation updates label Nov 25, 2019
Signed-off-by: Ben Johnson <bjohnson@binarylogic.com>
Signed-off-by: Ben Johnson <bjohnson@binarylogic.com>
@binarylogic binarylogic merged commit 357bdbb into vectordotdev:master Nov 25, 2019
@jamessewell
Copy link
Copy Markdown
Contributor Author

Amazing - sorry I didn’t get this one over the line myself!

@lukesteensen
Copy link
Copy Markdown
Member

No worries, thanks for getting it started!

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.

3 participants