Skip to content

Conversation

@lsyldliu
Copy link
Contributor

@lsyldliu lsyldliu commented Aug 17, 2022

What is the purpose of the change

Fix UsingRemoteJarITCase failed in hadoop3, test passed in local machine.

This change aim to address the ClassNotFoundException of HdfsConfiguration when run test with hadoop3, the reason is that hadoop-hdfs module doesn't include transitive dependency of hadoop-hdfs-client, hadoop-hdfs-client is not in classpath, so we need to introduce this dependency explicitly in maven.

Verifying this change

This change is already covered by existing tests in UsingRemoteJarITCase.

@flinkbot
Copy link
Collaborator

flinkbot commented Aug 17, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@zentol
Copy link
Contributor

zentol commented Aug 17, 2022

Please explain the change, and document the reasoning with a comment.

@lsyldliu
Copy link
Contributor Author

@zentol Thanks for review, I've updated it.

<exclusions>
<exclusion>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-annotations</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this being excluded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I print mvn dependency in hadoop2.8.5, found jackson-annotations is not included in hadoop-hdfs-client, but it existed in hadoop3, so I exclude it to keep consistency.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if that's a good idea. At the end of the day hadoop 3 has a different dependency tree, and trying to keep the close may just cause problems in the future when something suddenly stops working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I remove it.

@zentol zentol merged commit 0be27a5 into apache:master Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants