HBASE-27802 Manage static javascript resources programatically #6864
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
|
I tested this with starting HBase in standalone mode in dev mode ( Master UI
Region Server UI
REST UI
Also built a package and tested the UI there as well. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I see that in the PR build some replication related tests failed. I think they are not related. |
|
Hi @ndimiduk and @NihalJain, What do you think about this approach? |
NihalJain
reviewed
Mar 27, 2025
NihalJain
approved these changes
Mar 27, 2025
Looks great to me, very clean. Easy to do a new upgrade going forward. Thanks for taking this up @PDavid. |
|
Just one request, could you please verify these jars don't leak into our assembly and also double check on whether the js are not bundled with our server, rest, thrift jars i.e. HBASE-28921 Avoid bundling hbase-webapps folder in default jars still works. |
Thanks, great idea, I'll do that. |
@NihalJain OK, double-checked and no webjar is packaged into the tarball, also .js or .css file made it's way into the server, rest, thrift jars. Built a binary tarball locally with the command: List contents of the binary tarball: Check if hbase-server has any JavaScript / CSS files: Check if hbase-rest has any JavaScript / CSS files: Check if hbase-thrift has any JavaScript / CSS files: |
ndimiduk
reviewed
Mar 31, 2025
| @@ -295,7 +295,7 @@ | |||
| <overwrite>true</overwrite> | |||
| <resources> | |||
| <resource> | |||
| <directory>../hbase-server/src/main/resources/hbase-webapps/static</directory> | |||
| <directory>../hbase-server/target/hbase-webapps/static</directory> | |||
There was a problem hiding this comment.
I know that this is how you found it, but does it make better sense to include an explicit dependency on the new webjars jar from hbase-rest? I doubt that we can remove the dependency on hbase-server, but it would be one fewer thing and make the dependency tree more obvious.
There was a problem hiding this comment.
Many thanks, I completely agree your point that here and now we will have and implicit dependency from hbase-rest to hbase-server which is not obvious from first look. Also agree that this is not the best and would be better to have an explicit dependency.
The problem is that if we directly add the webjars to the dependencies section of the hbase-rest pom.xml, then those webjars will be included (packaged) into the hbase-rest jar which - as far as I understand - we want to avoid.
Maybe we could duplicate the maven-dependency-plugin (and then the copy steps) usage also to hbase-rest but then this would introduce a lot of XML duplication.
Other solution could be to extract the downloading and extraction of the webjars to a separate sub-project and then hase-server, hbase-rest and hbase-thrift could depend on it.
But I'm not a Maven expert by an means so if you have any better ideas please feel free to propose them. :)
There was a problem hiding this comment.
Created a new Jira to improve this: https://issues.apache.org/jira/browse/HBASE-29355
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
also reference vega JS files without version numbers.
…ncy-plugin this way no need to have a flat copy using maven-antrun-plugin. Only for this we have to use newer maven-dependency-plugin.
PDavid
force-pushed
the
HBASE-27802-js-programatically-2
branch
from
91dc70b to
c3d9af6
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
Just rebased this on latest master branch - the PR build is now green. |
|
Hi @ndimiduk, |
|
Sorry for the delay. Sure, let's try this. I still feel like there's a better way to manage this dependency more explicitly, but this is an improvement over what we had. |
ndimiduk
approved these changes
May 7, 2025
|
Many thanks for the reviews. @NihalJain and @ndimiduk. 👍 Maybe if you don't mind I'd merge this PR as is and open a followup Jira ticket to make the dependency more explicit. Is this an acceptable solution? |
I'm fine with that. Let me merge and start the backport process. |
|
Okay let me see how backports go |
|
Oof, yeah, not event to branch-3 does it apply obviously. Do you mind handling the backports @PDavid ? |
|
OK, found a small issue with this patch: The popovers ("Metrics" menu) were not working on the UI. Created an addendum PR here: #7018 (I'll include the addendum fix in the backport PR-s.) |
This was referenced May 25, 2025
PDavid
added a commit
to PDavid/hbase
that referenced
this pull request
May 25, 2025
- HBASE-28388 Avoid index based field sorting in tablesorter apache#6779 - HBASE-27802 Manage static javascript resources programatically apache#6864
ndimiduk
pushed a commit
that referenced
this pull request
May 26, 2025
* HBASE-27802 Manage static javascript resources programatically (#6864) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org> (cherry picked from commit 2f9d9fc) * HBASE-27802 Manage static javascript resources programatically (addendum: Fix not working popovers on UI) Popovers were not working on the UI and the following JS error was logged to browser console: ``` Uncaught TypeError: i.createPopper is not a function ``` This is because popper.js was not available on the page for Bootstrap. Solution: We use bootstrap.bundle.min.js instead which includes both popper.js and Bootstrap. --------- Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org>
|
Hi @ndimiduk and @NihalJain, I see, you already found but just for the record here are the backport PR-s here:
These backport PR-s already includes the fix (addendum PR) #7018 |
NihalJain
pushed a commit
that referenced
this pull request
May 27, 2025
* HBASE-27802 Manage static javascript resources programatically (#6864) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org> (cherry picked from commit 2f9d9fc) * HBASE-27802 Manage static javascript resources programatically (addendum: Fix not working popovers on UI) Popovers were not working on the UI and the following JS error was logged to browser console: ``` Uncaught TypeError: i.createPopper is not a function ``` This is because popper.js was not available on the page for Bootstrap. Solution: We use bootstrap.bundle.min.js instead which includes both popper.js and Bootstrap. --------- Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org>
NihalJain
pushed a commit
that referenced
this pull request
May 27, 2025
* HBASE-27802 Manage static javascript resources programatically (#6864) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org> (cherry picked from commit 2f9d9fc) * HBASE-27802 Fix not working popovers on UI Popovers were not working on the UI and the following JS error was logged to browser console: ``` Uncaught TypeError: i.createPopper is not a function ``` This is because popper.js was not available on the page for Bootstrap. Solution: We use bootstrap.bundle.min.js instead which includes both popper.js and Bootstrap. --------- Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org>
NihalJain
pushed a commit
that referenced
this pull request
May 27, 2025
* HBASE-27802 Manage static javascript resources programatically (#6864) Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org> (cherry picked from commit 2f9d9fc) * HBASE-27802 Manage static javascript resources programatically (addendum: Fix not working popovers on UI) Popovers were not working on the UI and the following JS error was logged to browser console: ``` Uncaught TypeError: i.createPopper is not a function ``` This is because popper.js was not available on the page for Bootstrap. Solution: We use bootstrap.bundle.min.js instead which includes both popper.js and Bootstrap. --------- Signed-off-by: Nick Dimiduk <ndimiduk@apache.org> Signed-off-by: Nihal Jain <nihaljain@apache.org>
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.
As is
Currently, the static JavaScript, CSS resources need to be manually managed. That is, if any of these has to be updated, we will be required to download them from web and place them under the appropriate path. This can be cumbersome.
After
To be able to manage them with minimal manual effort we could manage them as part of build.
Approach
This solution is very similar to how we manage the websitestatic resources in our build: #6668
Download and extract the UI resources as webjars using the
maven-dependency-pluginin our build. It will only copy the required files to thehbase-server/target/hbase-webapps/staticdirectory.The REST and Thrift projects copies the static JavaScript, CSS resources from hbase-server - as before.
This way the no new dependency is packaged into the application, only the required JS, CSS files are copied and packaged.
Upgrading in the future
From now on upgrading the static JavaScript, CSS resources means to:
pom.xml.Upgrades
Also upgraded the following static resources to current latest version: