HBASE-28433 Modify the assembly to not include test jars and their tr… #6184
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This will require HBASE-28432 to be inplace first. Putting this up to give a fair idea on how changes would look before I start HBASE-28432. Also test failures of this PR would give a fair idea on what all should be moved in new module as we refactor as part of HBASE-28432. |
NihalJain
commented
Aug 30, 2024
NihalJain
commented
Aug 30, 2024
NihalJain
commented
Aug 30, 2024
This comment was marked as outdated.
This comment was marked as outdated.
|
UT failures will say if we are being too aggressive here! |
NihalJain
commented
Aug 30, 2024
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Nice to see progress here! |
This was referenced Sep 16, 2024
stoty
reviewed
Sep 18, 2024
NihalJain
force-pushed
the
HBASE-28433
branch
from
8c81402 to
adb7158
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…ansitive dependencies
NihalJain
force-pushed
the
HBASE-28433
branch
from
adb7158 to
46190f0
Compare
|
Rebased code as #6258 is merged |
This comment has been minimized.
This comment has been minimized.
- Revert change of removing lib/test jars
NihalJain
commented
Oct 15, 2024
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Diff of assembly directories, master vs master with this PR: and Size Comparison, master vs master with this PR:: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
stoty
reviewed
Oct 17, 2024
|
Thank you @stoty for reviewing the code, will have a look at comments and revert back soon. |
|
I'm not sure hbase-it needs to be reverted anymore. At first I thought that we shouldn't introduce this potenitally breaking change, but since this goes only into a new major version, maybe we should prioritize correctness over backwards compatibility with incorrectly set up downstream projects. Whatever you decide to do with hbase-it is fine by me. |
…nt, now we revert back to directly using cached_classpath.txt to load jars required during runtime (as before). NOTE: To ensure cached_classpath is same as before this change, we retain some jars (including test jars) in assembly pom.xml which we actually remove during tarball creation.
NihalJain
force-pushed
the
HBASE-28433
branch
from
6466128 to
555573c
Compare
Went ahead with changing scope of the hadoop-minicluster jars from compile to test for correctness. |
|
Summary of 4th commit: Only 1 review comment is pending: on checking whether *:test-jar is a safety check or if we really hit it. I have forgotten what was the case given I rasied v1 of this PR almost 2 months ago. Let me redo analysis and come back to you. |
|
Updated Info:
and
VS
Please let me know if I should verify anything else. |
This comment has been minimized.
This comment has been minimized.
NihalJain
commented
Oct 21, 2024
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
stoty
reviewed
Oct 24, 2024
|
Thank you so much @stoty for patiently reviewing this one. I will be merging this in next few hours, unless no new reviews/objections comes up. |
NihalJain
added a commit
to NihalJain/hbase
that referenced
this pull request
Nov 8, 2024
…ansitive dependencies (apache#6184) - Before this change, our assembly included test jars and test dependencies, which added unnecessary bloat to the assembly and classpath. This not only increases the distribution size but also potentially introduces CVE-prone JARs into the binary assemblies. - With this change, we modify the assembly to exclude these test jars and their dependencies. This will result in a leaner, more secure assembly with a faster startup time. - To ensure cached_classpath is same as before this change, we retain some jars (including test jars) in assembly pom.xml which we actually remove during tarball creation. Signed-off-by: Istvan Toth <stoty@apache.org> (cherry picked from commit 3fbe4fb)
NihalJain
added a commit
that referenced
this pull request
Nov 11, 2024
…ansitive dependencies (#6184) (#6452) - Before this change, our assembly included test jars and test dependencies, which added unnecessary bloat to the assembly and classpath. This not only increases the distribution size but also potentially introduces CVE-prone JARs into the binary assemblies. - With this change, we modify the assembly to exclude these test jars and their dependencies. This will result in a leaner, more secure assembly with a faster startup time. - To ensure cached_classpath is same as before this change, we retain some jars (including test jars) in assembly pom.xml which we actually remove during tarball creation. Signed-off-by: Istvan Toth <stoty@apache.org> (cherry picked from commit 3fbe4fb)
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Jan 21, 2025
…their transitive dependencies (apache#6184)" This reverts commit 3fbe4fb.
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Feb 4, 2025
…their transitive dependencies (apache#6184)" This reverts commit 3fbe4fb.
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Feb 27, 2025
…their transitive dependencies (apache#6184)" This reverts commit 3fbe4fb.
vinayakphegde
added a commit
to vinayakphegde/hbase
that referenced
this pull request
Mar 7, 2025
…their transitive dependencies (apache#6184)" This reverts commit 3fbe4fb.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ansitive dependencies