Skip to content

HBASE-24820 [hbase-thirdparty] Add jersey-hk2 when shading jersey and…#29

Merged
Apache9 merged 2 commits intoapache:masterfrom
Apache9:HBASE-24820
Aug 8, 2020
Merged

HBASE-24820 [hbase-thirdparty] Add jersey-hk2 when shading jersey and…#29
Apache9 merged 2 commits intoapache:masterfrom
Apache9:HBASE-24820

Conversation

@Apache9
Copy link
Copy Markdown
Contributor

@Apache9 Apache9 commented Aug 6, 2020

… bump jetty to 9.4.31

@Apache9 Apache9 self-assigned this Aug 6, 2020
@Apache9
Copy link
Copy Markdown
Contributor Author

Apache9 commented Aug 6, 2020

Finally I could get all the UTs under hbase-http to pass with the shaded jetty and jersey.

There are still two problems:

  1. Shaded plugin can not deal with some string literals.

https://github.com/eclipse/jetty.project/blob/450ba27947e13e66baa8cd1ce7e85a4461cacc1d/jetty-webapp/src/main/java/org/eclipse/jetty/webapp/WebAppContext.java#L137

The elements of WebAppContext.__dftServerClasses have '-' at the beginning and the shade plugin will not relocate them and cause trouble(although when shading source they are relocated...).

I do not know how to replace string literals in a class file, so the solution is to write a method in the hbase code base to do the replacement...

  1. The behavior of HttpServletResponse.sendError has been changed. In the past jetty will append the error message in the status line but in jetty 9.4 it will not, and cause the TestProxyUserSpnegoHttpServer.testProxyDisallowedForUnprivileged to fail.
    Seems introduced by this commit
    jetty/jetty.project@bde8646#diff-03fcd9b9e322e2bb4fdb46f2bbea27b3

Not sure if it is intenional or not... But anyway this could also be solved by changing the test, not a big deal.

@Apache9 Apache9 requested a review from busbey August 6, 2020 14:21
@petersomogyi
Copy link
Copy Markdown
Contributor

Can you try to add the following relocation rule to deal with - prefix?

<relocation>
  <pattern>-org.eclipse</pattern>
  <shadedPattern>-${shaded.prefix}.org.eclipse</shadedPattern>
</relocation>

Not sure if it is intenional or not... But anyway this could also be solved by changing the test, not a big deal.

Changing the test looks fine to me.

@Apache9
Copy link
Copy Markdown
Contributor Author

Apache9 commented Aug 7, 2020

Can you try to add the following relocation rule to deal with - prefix?

<relocation>
  <pattern>-org.eclipse</pattern>
  <shadedPattern>-${shaded.prefix}.org.eclipse</shadedPattern>
</relocation>

You're right, I'm testing exactly what you suggest here(but with org.eclipse.jetty) before you post this comment :)

It is not easy to view the string literals in class file so let me depend on it in hbase to see if it works.

@Apache9
Copy link
Copy Markdown
Contributor Author

Apache9 commented Aug 7, 2020

Good, it works! I mean relocating '-org.eclipse.jetty'.

Pushed a new commit to add the relocation rule. PTAL @petersomogyi

@Apache9 Apache9 merged commit d361fba into apache:master Aug 8, 2020
Sign up for free to 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.

2 participants