steveloughran commented on code in PR #5540:
URL: https://github.com/apache/hadoop/pull/5540#discussion_r1167526769
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIdentityProviders.java:
##########
@@ -61,7 +63,7 @@ public void testPluggableIdentityProvider() {
CommonConfigurationKeys.IPC_IDENTITY_PROVIDER_KEY,
IdentityProvider.class);
- assertTrue(providers.size() == 1);
+ assertEquals(1, providers.size());
Review Comment:
use assertJ
```
assertThat(providers)
.describedAs("provider list")
.hasSize(1);
```
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIdentityProviders.java:
##########
@@ -71,12 +73,20 @@ public void testPluggableIdentityProvider() {
@Test
public void testUserIdentityProvider() throws IOException {
UserIdentityProvider uip = new UserIdentityProvider();
- String identity = uip.makeIdentity(new FakeSchedulable());
+ FakeSchedulable fakeSchedulable = new FakeSchedulable();
+ String identity = uip.makeIdentity(fakeSchedulable);
// Get our username
UserGroupInformation ugi = UserGroupInformation.getCurrentUser();
String username = ugi.getUserName();
assertEquals(username, identity);
+
+ // FakeSchedulable doesn't override getCallerContext()
+ // accessing it should throw a NotImplementedException
+ NotImplementedException nie = assertThrows(
Review Comment:
our own LambdaTestUtils.intercept() does this with better reporting of
mismatch, including rethrowing the failing exception if there's no string match
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Schedulable.java:
##########
@@ -29,5 +30,9 @@
public interface Schedulable {
public UserGroupInformation getUserGroupInformation();
+ default CallerContext getCallerContext() {
+ throw new NotImplementedException("Code is not implemented");
Review Comment:
i worry about exposing commons.* classes in an API, as if we change what is
raised external code may break. Just raise java's UnsupportedOperationException
and mention this may happen in the javadocs
##########
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Schedulable.java:
##########
@@ -29,5 +30,9 @@
public interface Schedulable {
public UserGroupInformation getUserGroupInformation();
+ default CallerContext getCallerContext() {
Review Comment:
javadocs, even if the rest of the interface left them out
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
