Skip to content

HBASE-22262 Removed deprecated methods from Filter class - #162

Merged
HorizonNet merged 1 commit into
apache:masterfrom
ultratendency:HBASE-22262
Jun 9, 2019
Merged

HBASE-22262 Removed deprecated methods from Filter class#162
HorizonNet merged 1 commit into
apache:masterfrom
ultratendency:HBASE-22262

Conversation

@HorizonNet

@HorizonNetHorizonNet commented Apr 17, 2019

Copy link
Copy Markdown
Contributor

No description provided.

@Apache-HBase

Apache-HBase commented Apr 17, 2019

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
0reexec39Docker mode activated.
_ Prechecks _
+1hbaseanti0Patch does not have any anti-patterns.
+1@author0The patch does not contain any @author tags.
+1test4tests0The patch appears to include 3 new or modified test files.
_ master Compile Tests _
0mvndep27Maven dependency ordering for branch
-1mvninstall83root in master failed.
-1compile16hbase-client in master failed.
-1compile28hbase-server in master failed.
-1compile24hbase-rest in master failed.
+1checkstyle123master passed
+1shadedjars281branch has no errors when building our shaded downstream artifacts.
-1findbugs60hbase-client in master has 83 extant Findbugs warnings.
-1findbugs153hbase-server in master has 3 extant Findbugs warnings.
-1findbugs24hbase-rest in master failed.
-1javadoc14hbase-client in master failed.
-1javadoc18hbase-server in master failed.
_ Patch Compile Tests _
0mvndep13Maven dependency ordering for patch
-1mvninstall78root in the patch failed.
-1compile15hbase-client in the patch failed.
-1compile28hbase-server in the patch failed.
-1compile24hbase-rest in the patch failed.
-1javac15hbase-client in the patch failed.
-1javac28hbase-server in the patch failed.
-1javac24hbase-rest in the patch failed.
+1checkstyle35hbase-client: The patch generated 0 new + 352 unchanged - 26 fixed = 352 total (was 378)
+1checkstyle74hbase-server: The patch generated 0 new + 42 unchanged - 2 fixed = 42 total (was 44)
+1checkstyle13The patch passed checkstyle in hbase-rest
+1whitespace0The patch has no whitespace issues.
+1shadedjars279patch has no errors when building our shaded downstream artifacts.
-1hadoopcheck44The patch causes 10 errors with Hadoop v2.7.4.
-1hadoopcheck86The patch causes 10 errors with Hadoop v3.0.0.
-1findbugs26hbase-rest in the patch failed.
-1javadoc14hbase-client in the patch failed.
-1javadoc19hbase-server in the patch failed.
_ Other Tests _
-1unit16hbase-client in the patch failed.
-1unit28hbase-server in the patch failed.
-1unit25hbase-rest in the patch failed.
+1asflicense27The patch does not generate ASF License warnings.
2076
SubsystemReport/Notes
DockerClient=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/Dockerfile
GITHUB PR#162
Optional Testsdupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
unameLinux e1202aa03048 4.4.0-141-generic #167~14.04.1-Ubuntu SMP Mon Dec 10 13:20:24 UTC 2018 x86_64 GNU/Linux
Build toolmaven
Personality/testptch/patchprocess/precommit/personality/provided.sh
git revisionmaster / 8ec93ea
mavenversion: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
Default Java1.8.0_181
mvninstallhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-mvninstall-root.txt
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-compile-hbase-client.txt
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-compile-hbase-server.txt
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-compile-hbase-rest.txt
findbugsv3.1.11
findbugshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-findbugs-hbase-client-warnings.html
findbugshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-findbugs-hbase-server-warnings.html
findbugshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-findbugs-hbase-rest.txt
javadochttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-javadoc-hbase-client.txt
javadochttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/branch-javadoc-hbase-server.txt
mvninstallhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-mvninstall-root.txt
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-compile-hbase-client.txt
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-compile-hbase-server.txt
compilehttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-compile-hbase-rest.txt
javachttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-compile-hbase-client.txt
javachttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-compile-hbase-server.txt
javachttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-compile-hbase-rest.txt
hadoopcheckhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-javac-2.7.4.txt
hadoopcheckhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-javac-3.0.0.txt
findbugshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-findbugs-hbase-rest.txt
javadochttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-javadoc-hbase-client.txt
javadochttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-javadoc-hbase-server.txt
unithttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-unit-hbase-client.txt
unithttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-unit-hbase-server.txt
unithttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/artifact/out/patch-unit-hbase-rest.txt
Test Resultshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/testReport/
Max. process+thread count83 (vs. ulimit of 10000)
modulesC: hbase-client hbase-server hbase-rest U: .
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/1/console
Powered byApache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@Apache-HBase

Apache-HBase commented Apr 19, 2019

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
0reexec24Docker mode activated.
_ Prechecks _
+1hbaseanti0Patch does not have any anti-patterns.
+1@author0The patch does not contain any @author tags.
+1test4tests0The patch appears to include 3 new or modified test files.
_ master Compile Tests _
0mvndep27Maven dependency ordering for branch
+1mvninstall276master passed
+1compile110master passed
+1checkstyle128master passed
+1shadedjars284branch has no errors when building our shaded downstream artifacts.
+1findbugs300master passed
+1javadoc84master passed
_ Patch Compile Tests _
0mvndep14Maven dependency ordering for patch
+1mvninstall252the patch passed
+1compile106the patch passed
+1javac106the patch passed
+1checkstyle35hbase-client: The patch generated 0 new + 352 unchanged - 26 fixed = 352 total (was 378)
+1checkstyle70hbase-server: The patch generated 0 new + 42 unchanged - 2 fixed = 42 total (was 44)
+1checkstyle17The patch passed checkstyle in hbase-rest
+1whitespace0The patch has no whitespace issues.
+1shadedjars271patch has no errors when building our shaded downstream artifacts.
+1hadoopcheck559Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
+1findbugs335the patch passed
+1javadoc76the patch passed
_ Other Tests _
+1unit202hbase-client in the patch passed.
-1unit7563hbase-server in the patch failed.
+1unit266hbase-rest in the patch passed.
+1asflicense92The patch does not generate ASF License warnings.
11199
ReasonTests
Failed junit testshadoop.hbase.security.token.TestZKSecretWatcher
SubsystemReport/Notes
DockerClient=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/2/artifact/out/Dockerfile
GITHUB PR#162
Optional Testsdupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
unameLinux 9ce6db6b80b5 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24 10:58:50 UTC 2018 x86_64 GNU/Linux
Build toolmaven
Personality/testptch/patchprocess/precommit/personality/provided.sh
git revisionmaster / 268bcce
mavenversion: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
Default Java1.8.0_181
findbugsv3.1.11
unithttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/2/artifact/out/patch-unit-hbase-server.txt
Test Resultshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/2/testReport/
Max. process+thread count4685 (vs. ulimit of 10000)
modulesC: hbase-client hbase-server hbase-rest U: .
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/2/console
Powered byApache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@HorizonNet

HorizonNet commented Apr 19, 2019

Copy link
Copy Markdown
ContributorAuthor

The test failure seems to be unrelated to the actual change.

@Apache-HBase

Apache-HBase commented Apr 20, 2019

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
0reexec23Docker mode activated.
_ Prechecks _
+1hbaseanti0Patch does not have any anti-patterns.
+1@author0The patch does not contain any @author tags.
+1test4tests0The patch appears to include 3 new or modified test files.
_ master Compile Tests _
0mvndep29Maven dependency ordering for branch
+1mvninstall260master passed
+1compile109master passed
+1checkstyle121master passed
+1shadedjars269branch has no errors when building our shaded downstream artifacts.
+1findbugs284master passed
+1javadoc81master passed
_ Patch Compile Tests _
0mvndep17Maven dependency ordering for patch
+1mvninstall244the patch passed
+1compile102the patch passed
+1javac102the patch passed
+1checkstyle33hbase-client: The patch generated 0 new + 352 unchanged - 26 fixed = 352 total (was 378)
+1checkstyle67hbase-server: The patch generated 0 new + 42 unchanged - 2 fixed = 42 total (was 44)
+1checkstyle16The patch passed checkstyle in hbase-rest
+1whitespace0The patch has no whitespace issues.
+1shadedjars265patch has no errors when building our shaded downstream artifacts.
+1hadoopcheck508Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
+1findbugs337the patch passed
+1javadoc77the patch passed
_ Other Tests _
+1unit199hbase-client in the patch passed.
+1unit7596hbase-server in the patch passed.
+1unit266hbase-rest in the patch passed.
+1asflicense90The patch does not generate ASF License warnings.
11095
SubsystemReport/Notes
DockerClient=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/3/artifact/out/Dockerfile
GITHUB PR#162
Optional Testsdupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
unameLinux 716c71e53a49 4.4.0-139-generic #165-Ubuntu SMP Wed Oct 24 10:58:50 UTC 2018 x86_64 GNU/Linux
Build toolmaven
Personality/testptch/patchprocess/precommit/personality/provided.sh
git revisionmaster / f30d6c9
mavenversion: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
Default Java1.8.0_181
findbugsv3.1.11
Test Resultshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/3/testReport/
Max. process+thread count4726 (vs. ulimit of 10000)
modulesC: hbase-client hbase-server hbase-rest U: .
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/3/console
Powered byApache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

saintstack
saintstack reviewed Apr 23, 2019
View reviewed changes

@saintstacksaintstack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For sure deprecated for a whole major version? Deprecate in hbase 2.0.0 release? Thanks.

@Apache9

Apache9 commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

For sure deprecated for a whole major version? Deprecate in hbase 2.0.0 release?

I think 'a whole major version' just means we can not remove deprecated stuff if it is first deprecated in the same major version? For example, it is OK to remove a class in 3.0.0 if it is deprecated in 2.1.0, but you can not remove it in 2.2.0.

@saintstack

saintstack commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

"An API needs to be deprecated for a major version before we will change/remove it."

... in "Client API compatibility" in http://hbase.apache.org/book.html#hbase.versioning.post10

@HorizonNet

HorizonNet commented Apr 23, 2019

Copy link
Copy Markdown
ContributorAuthor

I had the same understanding as @Apache9. I know we had a discussion about this some time ago, but wasn't able to find it anymore. Probably the documentation needs some more details (an example would be good), because it is up to some kind of interpretation. I think if we deprecate something in 2.0.0 we can remove it in 3.0.0. But what about the minor versions? If we deprecated something in 2.3.0 and the next version to be released is 3.0.0 we don't really have a deprecation for a major version. Removing it in 3.3.0 also doesn't seem to be good. With that the next major version would be 4.0.0, so removing it after 1.5 major versions.

@saintstack

saintstack commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

My interpretation is that you have to wait at least a major version so as per your example, it'd be 4.0.0 if deprecation was added in anything after 2.0.0.

@Apache9

Apache9 commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

My interpretation is that you have to wait at least a major version so as per your example, it'd be 4.0.0 if deprecation was added in anything after 2.0.0.

That would be too tough? I think a major release is where we do breaking changes. And I found this in our ref guide

Example: A user using a newly deprecated API does not need to modify application code with HBase API calls until the next major version. *

So I think it is OK to remove deprecated classes in a major release? We can leave it there for some reasons, but removing it is also acceptable.

@saintstack

saintstack commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

Sounds like we need to get clarification here. I think the 'Example' sentence is open to various interpretation. ML?

@Apache9

Apache9 commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

I have already done some removal of deprecated classes on master...

But I think your point on 'at lease a whole major release' is also valid. For example, we have deprecated something on 2.1.0, but users may upgrade to 3.0.0 from 2.0.0, so they will not see the deprecation and find out some APIs are gone...

So maybe we need a guide on rolling upgrading, for example, 2.x users must upgrade to, for example, 2.3.0 first, before upgrading to 3.0.0? And we must make sure that, all classes deprecated after 2.3.0 release, must be retained until we release 4.0.0.

What do you think?

@saintstack

saintstack commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

Lets discuss on ML. I think asking folks to rolling upgrade to the most minor version before going to next major may be an unfair expectation.

@Apache9

Apache9 commented Apr 23, 2019

Copy link
Copy Markdown
Contributor

Anyway we have already done lots of removal and it is not likely that we can add them back, so...

@HorizonNet

HorizonNet commented Apr 23, 2019

Copy link
Copy Markdown
ContributorAuthor

Also agree to discuss it on the ML. Will create a thread later today. We definitely need to make the documentation more clear. Regarding this PR: The two methods in the Filter class were deprecated in 2.0.0, so we should be fine to remove them for 3.0.0.

@Apache-HBase

Apache-HBase commented Apr 25, 2019

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
0reexec267Docker mode activated.
_ Prechecks _
+1hbaseanti0Patch does not have any anti-patterns.
+1@author0The patch does not contain any @author tags.
+1test4tests0The patch appears to include 3 new or modified test files.
_ master Compile Tests _
0mvndep26Maven dependency ordering for branch
+1mvninstall273master passed
+1compile109master passed
+1checkstyle121master passed
+1shadedjars262branch has no errors when building our shaded downstream artifacts.
+1findbugs293master passed
+1javadoc72master passed
_ Patch Compile Tests _
0mvndep26Maven dependency ordering for patch
+1mvninstall269the patch passed
+1compile106the patch passed
+1javac106the patch passed
+1checkstyle35hbase-client: The patch generated 0 new + 340 unchanged - 26 fixed = 340 total (was 366)
+1checkstyle69hbase-server: The patch generated 0 new + 42 unchanged - 2 fixed = 42 total (was 44)
+1checkstyle15The patch passed checkstyle in hbase-rest
+1whitespace0The patch has no whitespace issues.
+1shadedjars269patch has no errors when building our shaded downstream artifacts.
+1hadoopcheck545Patch does not cause any errors with Hadoop 2.7.4 or 3.0.0.
+1findbugs336the patch passed
+1javadoc72the patch passed
_ Other Tests _
+1unit205hbase-client in the patch passed.
-1unit17532hbase-server in the patch failed.
+1unit490hbase-rest in the patch passed.
+1asflicense74The patch does not generate ASF License warnings.
21583
ReasonTests
Failed junit testshadoop.hbase.master.procedure.TestSCPWithReplicas
hadoop.hbase.client.TestAdmin1
hadoop.hbase.master.procedure.TestSCPWithReplicasWithoutZKCoordinated
hadoop.hbase.client.TestConnectionImplementation
hadoop.hbase.client.TestSnapshotTemporaryDirectoryWithRegionReplicas
hadoop.hbase.client.TestFromClientSide3
hadoop.hbase.master.TestSplitWALManager
hadoop.hbase.client.replication.TestReplicationAdminWithClusters
hadoop.hbase.client.TestFromClientSide
hadoop.hbase.client.TestHbck
SubsystemReport/Notes
DockerClient=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/4/artifact/out/Dockerfile
GITHUB PR#162
Optional Testsdupname asflicense javac javadoc unit findbugs shadedjars hadoopcheck hbaseanti checkstyle compile
unameLinux 4e273caea6f0 4.4.0-137-generic #163-Ubuntu SMP Mon Sep 24 13:14:43 UTC 2018 x86_64 GNU/Linux
Build toolmaven
Personality/testptch/patchprocess/precommit/personality/provided.sh
git revisionmaster / 184568c
mavenversion: Apache Maven 3.5.4 (1edded0938998edf8bf061f1ceb3cfdeccf443fe; 2018-06-17T18:33:14Z)
Default Java1.8.0_181
findbugsv3.1.11
unithttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/4/artifact/out/patch-unit-hbase-server.txt
Test Resultshttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/4/testReport/
Max. process+thread count5138 (vs. ulimit of 10000)
modulesC: hbase-client hbase-server hbase-rest U: .
Console outputhttps://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-162/4/console
Powered byApache Yetus 0.9.0 http://yetus.apache.org

This message was automatically generated.

@saintstack

saintstack commented Jun 7, 2019

Copy link
Copy Markdown
Contributor

So, we got clarification on the ML. How does it effect the patch? Thanks.

@HorizonNet

HorizonNet commented Jun 9, 2019

Copy link
Copy Markdown
ContributorAuthor

@saintstack It shouldn't affect the patch, because the two methods were deprecated in 2.0.0 and with that can be removed in 3.0.0. In my opinion we're ready to merge.

@saintstack

saintstack commented Jun 9, 2019

Copy link
Copy Markdown
Contributor

@HorizonNet Ok. +1 on commit sir.

@HorizonNet
HorizonNet merged commit f620285 into apache:masterJun 9, 2019
@HorizonNet
HorizonNet deleted the HBASE-22262 branch June 9, 2019 22:47
ndimiduk pushed a commit to ndimiduk/hbase that referenced this pull request Jun 2, 2025
@hgromerHernan Gelaf-Romer
fix backup location (apache#162)
5a2b53b
Co-authored-by: Hernan Gelaf-Romer <hgelafromer@hubspot.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@saintstacksaintstacksaintstack approved these changes

Assignees

No one assigned

Labels

enhancement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@HorizonNet@Apache-HBase@Apache9@saintstack