-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[ZEPPELIN-6253] fix: remove SNAPSHOT from version when building crede… #4993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing the broken doc URL.
Just to clarify, even after removing -SNAPSHOT
, the docs page for the current development version might still be missing, since the latest version is not released yet. So the link could still be broken unless it falls back to a stable version like latest
.
We might also consider a unified approach in #4980 if needed.
let currentVersion = $rootScope.zeppelinVersion || '0.12.0'; | ||
currentVersion = currentVersion.replace('-SNAPSHOT', ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about replacing the -SNAPSHOT
part if present, and falling back to 'latest'
instead?
Also, I think it's preferrable to avoid hardcoding version strings like '0.12.0'
.
let currentVersion = $rootScope.zeppelinVersion || '0.12.0'; | |
currentVersion = currentVersion.replace('-SNAPSHOT', ''); | |
const currentVersion = $rootScope.zeppelinVersion | |
&& $rootScope.zeppelinVersion.replace('-SNAPSHOT', '') || 'latest'; |
|
||
/* | ||
* Add '/setup' to doc link on the version over 0.7.0 | ||
* Add '/setup' to doc link on the version over 0.7.0, add '/setup' to the doc URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The add part seems to repeat the same meaning as the original comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review!
I updated the code to remove the hardcoded default version and added a fallback to 'latest' when zeppelinVersion is not available.
Also simplified the comment to avoid duplication.
…' in credential docs link
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the PR title might be unfinished — could you please review it and update if needed?
What is this PR for?
Fixes a broken "Learn more" link on the credential page.
Zeppelin currently appends
-SNAPSHOT
to the version when constructing the documentation link, which results in a 404 page.This PR removes
-SNAPSHOT
from the version and ensures it uses a valid documentation path.Broken link (before):
https://zeppelin.apache.org/docs/0.12.0-SNAPSHOT/setup/security/datasource_authorization.html
Corrected link (after):
https://zeppelin.apache.org/docs/0.12.0/setup/security/datasource_authorization.html
What type of PR is it?
What is the Jira issue?
JIRA: ZEPPELIN-6253
How should this be tested?
Questions