Skip to content

Conversation

haizhou
Copy link
Contributor

@haizhou haizhou commented Nov 11, 2014

We have fields defined as a date type in the mapping, and we store the millisecond long timestamp for those fields. However, when we are streaming using mapreduce, we get exception saying Date cannot be cast as a Writable.

Initial investigation shows that the WritableValueReader class does not map date values coming from its super class JdkValueReader to a hadoop writable type.

@costin costin closed this in 00a1adc Nov 16, 2014
@costin
Copy link
Member

costin commented Nov 16, 2014

Thanks for the report @haizhou. Since you didn't sign the CLA and to time constraints (release schedule) I could not use your PR - please make sure to sign the CLA if you plan to contribute in the future (and I hope you do).

Cheers!

@mgreene
Copy link

mgreene commented Nov 17, 2014

@costin EverTrue has signed that document, organizationally speaking, we should be all set.

@costin
Copy link
Member

costin commented Nov 17, 2014

Thanks - I wasn't aware of the organizational sign-up - I checked the user address/email directly. I'll make a note to remember the next time around.

@mgreene
Copy link

mgreene commented Nov 17, 2014

I was under the impression that work could not be merged unless that agreement was signed in the first place?

For the future, I think it would be better for the community to solicit that signature for that document first before committing to master under your account so that credit can be properly attributed in the tree.

@costin
Copy link
Member

costin commented Nov 17, 2014

@mgreene The PR/work was not merged or reviewed since the CLA was not in place as I have indicated (based on @haizhou profile/email check) in my previous answer.
I chose to fix the bug based on the description offered in the PR and since it was trivial (1 line of code - 3 lines total). The reason I did not wait for the confirmation was the immediate release of 2.1.0.Beta3 - if you look through the rest of the issues, you'll notice I typically wait for an answer however in this case, as it was a trivial fix and the release was pending, I chose to pull the trigger.

Going forward, it would help a lot to figure out from the start whether a contributor signed the CLA or not.
In this case, there's nothing in @haizhou profile that indicates he's part of EverTrue (nor his email or group organization).
Further more, I just asked on whether the organizational level CLA is good enough and it seems it is not; a contributor CLA is also needed to avoid conflict (between his company and the contributor himself).
In other words, if @haizhou signs the CLA we're all set across the entire Elasticsearch projects (including es-hadoop).

I am sorry if you feel credit hasn't been given - I'll be sure to use this PR for the 2.0.x branch or reapply it to 2.1.x. once @haizhou CLA is in place.

Cheers,

@haizhou
Copy link
Contributor Author

haizhou commented Nov 19, 2014

@costin what are the steps for me to get a copy of the CLA?

@costin
Copy link
Member

costin commented Nov 20, 2014

@haizhou See https://github.com/elasticsearch/elasticsearch-hadoop/blob/master/CONTRIBUTING.md - it appears as a header each time a PR is created.
Thanks.

@costin costin reopened this Nov 20, 2014
@haizhou
Copy link
Contributor Author

haizhou commented Nov 20, 2014

@costin thanks for the link, and i just proceeded and signed the CLA.

@haizhou haizhou changed the title Fix long dates Fix writable serialization of date type with long values and nested type Nov 20, 2014
@haizhou
Copy link
Contributor Author

haizhou commented Nov 20, 2014

@costin please also note the newest commit about the nested type serialization. I was trying to submit a separate pull request, but it seems github would only allow me to use the existing one from a forked repo.

@costin
Copy link
Member

costin commented Nov 20, 2014

Thanks. It's best to create a branch per PR as that allows the same
repository to be used for multiple PRs. If I'm not mistaken there are some
posts about this on the github blog.

Cheers

Costin
On Nov 20, 2014 6:15 PM, "Hai Zhou" notifications@github.com wrote:

@costin https://github.com/costin please also note the newest commit
about the nested type serialization. I was trying to submit a separate pull
request, but it seems github would only allow me to use the existing one
from a forked repo.


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

@costin
Copy link
Member

costin commented Nov 20, 2014

Great. Thank you! I'll try to review and merge the PRs early next week to
once I return from Strata. Cheers.

Costin
On Nov 20, 2014 5:59 PM, "Hai Zhou" notifications@github.com wrote:

@costin https://github.com/costin thanks for the link, and i just
proceeded and signed the CLA.


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

@haizhou haizhou changed the title Fix writable serialization of date type with long values and nested type Fix writable serialization of date type with long values Nov 21, 2014
@haizhou
Copy link
Contributor Author

haizhou commented Nov 21, 2014

Submitted #327 for the other issue. Closing this one now.

@costin
Copy link
Member

costin commented Dec 11, 2014

@haizhou can you confirm the email you have used to sign the CLA? It needs to be same one as your Github account and currently islandsword@gmail.com does not appear in the list...

@haizhou
Copy link
Contributor Author

haizhou commented Dec 11, 2014

@costin I just changed my github account email. It should be matching what I used to sign the CLA now.

@costin
Copy link
Member

costin commented Dec 11, 2014

@haizhou Unfortunately you'll have to resubmit the PR since your git commits are tied to your previous emails. Or potentially use git to rewrite the commit author (should be easier).

@haizhou
Copy link
Contributor Author

haizhou commented Dec 11, 2014

@costin I just signed the CLA again under the other email. Please confirm.

@costin
Copy link
Member

costin commented Dec 12, 2014

@haizhou Thanks for signing and your patience. I've merged #327 into master and 2.0.x. Unfortunately for this PR, #320, it looks like your contributing branch has been updated and no commits are available. Thus there is nothing to re-apply/merge...
Let me know how you want to proceed going forward.

Cheers,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants