-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[parquet-thrift] start removing things marked for deprecation #3318
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
base: master
Are you sure you want to change the base?
Conversation
Some build errors I'll chase down when 2.0.0 is happening |
94cf4be
to
f8c4006
Compare
Pushed several test fixes and removed some unneeded dependencies |
String filterDesc, TBase toWrite, TBase toRead, Class<? extends TBase<?, ?>> thriftClass) throws Exception { | ||
Configuration conf = new Configuration(); | ||
conf.set(ThriftReadSupport.THRIFT_COLUMN_FILTER_KEY, filterDesc); | ||
conf.set(ThriftReadSupport.STRICT_THRIFT_COLUMN_FILTER_KEY, filterDesc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file was exclusively testing the deprecated thrift filter. I switched it to test the supported "strict" filter but could not recreate the functionality of several of the tests. This isn't really a regression in test coverage since the strict filter was not being tested at all.
Thank you for running the tests @gszadovszky , I was able to track down the problems and find some more things to remove. |
I don't understand this build error:
|
I think the error |
I'm not sure about this error either. But right, when we actually switch master for 2.0 development, we should disable japicmp until the first release. Or maybe it is intelligent enough to realized a major version change if we change it to 2.0.0-SNAPSHOT... |
A 2.0.0 release is being discussed so I took a pass at removing everything marked for deprecation in
parquet-thrift
. Some of the deprecations have been there for 10 years.