Skip to content

Conversation

@rubanm
Copy link
Contributor

@rubanm rubanm commented Feb 1, 2016

This PR drops our dependency on parquet-cascading for easier cascading upgrades in the future (decoupled from apache's parquet release process).

As a follow-up to this change, I will copy over @cchepelov's cascading3 changes from apache/parquet-java#284 and file an issue on parquet-mr to possibly remove their version of this code (presumably, no one outside of scalding consumes this).

I copied over the java code instead of translating to scala so that

  1. We do not lose anything in translation
  2. Cyrille's already done work for cascading3 can be easily ported
  3. A majority of the code here is just wiring things down to cascading's layer which imo wouldn't give us much in the scala version.

But let me know if anyone feels strongly about creating a scala version of these schemes instead.

cc @ianoc @isnotinvain @johnynek @julienledem

Copy link
Collaborator

Choose a reason for hiding this comment

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

why don't we need this exclude?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added back the exclude. There was an unused pig import in the original code which we don't need.

@julienledem
Copy link
Contributor

@rubanm sounds good to me.

@rubanm
Copy link
Contributor Author

rubanm commented Feb 2, 2016

This passes Twitter internal e2e tests.

@johnynek
Copy link
Collaborator

johnynek commented Feb 2, 2016

👍 basically this is becoming a scalding mono-repo...

@rubanm
Copy link
Contributor Author

rubanm commented Feb 2, 2016

Hah, with monobuild first this time around.

ianoc added a commit that referenced this pull request Feb 2, 2016
@ianoc ianoc merged commit e60da71 into twitter:develop Feb 2, 2016
@rubanm rubanm deleted the rubanm/drop_parquet_cascading branch February 2, 2016 16:30
rubanm pushed a commit that referenced this pull request Feb 9, 2016
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.

4 participants