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

Adds a typedjson source #1129

Merged
merged 6 commits into from
Dec 10, 2014
Merged

Adds a typedjson source #1129

merged 6 commits into from
Dec 10, 2014

Conversation

ianoc
Copy link
Collaborator

@ianoc ianoc commented Dec 9, 2014

So this involved adding json4s and elephantbird into the json package. Not sure if we mind that?

@isnotinvain
Copy link
Contributor

It's not a showstopper, can't see how it would cause trouble, just wondering why we need two json libraries.

@ianoc
Copy link
Collaborator Author

ianoc commented Dec 10, 2014

Well json4s is just nicer for scala, and we've tested that it works well internally.

import Dsl._
import TypedJson._

val fieldSym = 'jsonString
Copy link
Collaborator

Choose a reason for hiding this comment

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

can this be private?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess to use this with the Fields API you'd need it. Nevermind.

@johnynek
Copy link
Collaborator

merge when green.

ianoc added a commit that referenced this pull request Dec 10, 2014
@ianoc ianoc merged commit 111bbda into develop Dec 10, 2014
@ianoc ianoc deleted the typedJsonSource branch December 10, 2014 19:21
@isnotinvain
Copy link
Contributor

I took a stab at using jackson last night just to see what's involved. Switching to the jackson API was easy, but I got hung up on adding a JobTest that exercises the source -- if transformInTest is false, then the source does no work, so there's nothing to test. If transformInTest is true, then it gets confused and tries to treat a TypedPipe[MyCaseClass] as a TypedPipe[String] and then says this is invalid json.

Anyway, probably not a huge deal, if we get classpath issues later we can revisit (json4s can apparently be backed by jackson, not sure how that's registered of if having out of sync json4s / jackson on the classpath is an issue)

@johnynek
Copy link
Collaborator

Just trying at the REPL is fine. You should be able to write, close the
repl, then open and read.

On Wed, Dec 10, 2014 at 9:59 AM, Alex Levenson notifications@github.com
wrote:

I took a stab at using jackson last night just to see what's involved.
Switching to the jackson API was easy, but I got hung up on adding a
JobTest that exercises the source -- if transformInTest is false, then
the source does no work, so there's nothing to test. If transformInTest
is true, then it gets confused and tries to treat a TypedPipe[MyCaseClass]
as a TypedPipe[String] and then says this is invalid json.

Anyway, probably not a huge deal, if we get classpath issues later we can
revisit (json4s can apparently be backed by jackson, not sure how that's
registered of if having out of sync json4s / jackson on the classpath is an
issue)


Reply to this email directly or view it on GitHub
#1129 (comment).

Oscar Boykin :: @posco :: http://twitter.com/posco

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

5 participants