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

[#1066] Fix issues with reading from the Accumulo Store. #1145

Merged
merged 17 commits into from Feb 20, 2019

Conversation

Projects
None yet
3 participants
@p-f
Copy link
Collaborator

p-f commented Jan 15, 2019

Relocate all uses of Kryo and it's major dependencies.
Flink also uses Kryo in a different version, which will lead to issues because incompatible versions of Kryo are mixed during execution.

Also adds the Objenesis library to the JARs, since it is needed by Kryo (see #1130).

With this solution, Kryo + it's dependencies has to be added to the gradoop-accumulo artifact, since references to Kryo classes will be changed too.

Also change the kryo-shaded dependency to kryo, as those artifact are the same thing (as far as I've seen).

Needs testing on a cluster.

Warning: This includes major changes to the build process of the gradoop-accumulo module. Please test if uploading to repos still works for this artifact.

Another approach would be to use Flink's version of Kryo and manually set the classloader to use Accumulo's loader. (This seems a bit like a dirty workaround to me.)

fixes #1066 (hopefully)

p-f and others added some commits Dec 14, 2018

@p-f

This comment has been minimized.

Copy link
Collaborator Author

p-f commented Jan 16, 2019

There may be changes necessary to the NOTICE file, since the gradoop-accumulo JAR bundles some dependencies with this change.
I'm trying to remove those from the main JAR and only include them in the iterator JAR.

p-f and others added some commits Jan 16, 2019

@ChrizZz110

This comment has been minimized.

Copy link
Contributor

ChrizZz110 commented Feb 20, 2019

@p-f Kryo is under BSD-3 license. Please add an section in our LICENSE file below "The Apache License 2.0" named 3-Clause BSD License and Add Kryo with it's version. Put objenesis to the Apache section. Then we will be safe.

p-f and others added some commits Feb 20, 2019

@galpha
Copy link
Contributor

galpha left a comment

LGTM - only one minor change

LICENSE Outdated

-----------------------------------------------------------------------
Copyright notice for Minilog 1.3.0

This comment has been minimized.

@galpha

galpha Feb 20, 2019

Contributor

The copyright notice for minilog and kryo are identical (except the mark down highlighting). Remove one block and extend the notice header as follows: "Copyright notice for Kryo 4.0.2 and Minilog 1.3.0"

This comment has been minimized.

@p-f

p-f Feb 20, 2019

Author Collaborator

They are not identical imo, Kryo is (c) 2008 - 2013 while Minilog is just (c) 2008

@p-f

This comment has been minimized.

Copy link
Collaborator Author

p-f commented Feb 20, 2019

I moved licenses to the correct location (similar to what is done with Flink).
We should consider reorganizing licenses in general (after pr).

@galpha

galpha approved these changes Feb 20, 2019

@galpha galpha merged commit f602c62 into dbs-leipzig:develop Feb 20, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.