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

add ability to export as newline separated JSON (nsj) #5101

Merged
merged 4 commits into from Nov 17, 2015

Conversation

tayloramurphy
Copy link

Added ability to export as newline separated JSON (nsj) as well as appropriate language.

Also added language to import.py that indicates nsj is already accepted as an import type.

@@ -298,14 +306,15 @@ def csv_writer(filename, fields, delimiter, task_queue, error_queue):
pass

def launch_writer(format, directory, db, table, fields, delimiter, task_queue, error_queue):
if format == "json":
if format == "json" or format == "nsj":
filename = directory + "/%s/%s.json" % (db, table)
Copy link
Member

Choose a reason for hiding this comment

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

I think the file extension should reflect the data type - since the file can't be parsed as valid JSON, it shouldn't be named with a .json.

@Tryneus
Copy link
Member

Tryneus commented Nov 16, 2015

@tayloramurphy, this looks pretty good to me, thanks for the pull request!

I think nsj is a little strange for the name of the format. The closest thing I could find to a standard for this format was http://jsonlines.org/ (which suggests jsonl) or http://dataprotocols.org/ndjson/ (which suggests ndjson). I think renaming it to ndjson would be a little cleaner and more apparent to users.

@tayloramurphy
Copy link
Author

I like ndjson. I'll update the PR.

Thanks for the input!

@tayloramurphy
Copy link
Author

@Tryneus Updated appropriately. It looks better as ndjson for sure.

@Tryneus
Copy link
Member

Tryneus commented Nov 16, 2015

@tayloramurphy, awesome! I almost forgot - one last thing before we can merge it, could you sign our CLA at http://rethinkdb.com/community/cla/?

@tayloramurphy
Copy link
Author

Done. I tried to do it earlier but it wouldn't load properly b/c of mixed content types. Seems to have submitted properly this time.

@Tryneus
Copy link
Member

Tryneus commented Nov 17, 2015

@tayloramurphy, cool, merging. Thanks again for the pull request!

Tryneus added a commit that referenced this pull request Nov 17, 2015
add ability to export as newline separated JSON (nsj)
@Tryneus Tryneus merged commit e1c4286 into rethinkdb:next Nov 17, 2015
@danielmewes danielmewes added this to the 2.3 milestone Nov 17, 2015
@tayloramurphy tayloramurphy deleted the nsjexport branch November 18, 2015 03:23
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