Skip to content

HDDS-2444. Remove server side dependencies from ozonefs jar files - #130

Closed
elek wants to merge 2 commits into
apache:masterfrom
elek:HDDS-2444
Closed

HDDS-2444. Remove server side dependencies from ozonefs jar files#130
elek wants to merge 2 commits into
apache:masterfrom
elek:HDDS-2444

Conversation

@elek

@elekelek commented Nov 8, 2019

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

During the review of HDDS-2427 we found that some of the server side dependencies (container-service, framework) are added to the ozonefs library jars. Server side dependencies should be excluded from the client side to make the client safer and the build faster.

This patch doesn't introduce any new features just a dependency cleanup:

  1. server side dependencies removed from hadoop-ozone/common
  2. explicit server side dependencies added to the projects where they are used (eg. s3g)
  3. Some static utility methods are reorganized to make them available

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-2444

How was this patch tested?

Compiled and started basic acceptance test (compose/ozone/test.sh)

@bharatviswa504bharatviswa504 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.

I think we can remove the change added in HDDS-2427 right?
As now ozone-common does not bring in hdds-framework and hadoop-hdds-container-service.

@elek

elek commented Nov 11, 2019

Copy link
Copy Markdown
MemberAuthor

I think we can remove the change added in HDDS-2427 right?
As now ozone-common does not bring in hdds-framework and hadoop-hdds-container-service.

Right, it's not required any more. I added the removal to this patch.

@bharatviswa504bharatviswa504 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.

+1 LGTM. CI issue needs to be fixed.
Not sure if test failures are related to this change.

@bharatviswa504

Copy link
Copy Markdown
Contributor

/retest

@anuengineer

Copy link
Copy Markdown
Contributor

+1. From me too. There is a Rat Check failure. You can fix that push and commit.

@dineshchitlangiadineshchitlangia 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.

Overall LGTM, just need to fix the License issue and missing newline at EOF.

Revert "HDDS-2427. Exclude webapps from hadoop-ozone-filesystem-lib-current uber jar"
This reverts commit 6a450fb.
@elek

elek commented Dec 3, 2019

Copy link
Copy Markdown
MemberAuthor

Thanks @bharatviswa504, @anuengineer and @dineshchitlangia the review. The comments are addressed and last build show that the rat is fine.

Acceptance tests are unrelated and will be fixed by #282. I will merge this patch soon...

@elekelek closed this in 46fd24bDec 3, 2019
Sign up for freeto 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

@elek@bharatviswa504@anuengineer@dineshchitlangia