Skip to content
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.

Sign up for GitHub

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

[#5976] Improvement(bin):Add validation checks to the startup scripts to prevent incorrect usage #5977

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[#5976] Improvement(bin):Add validation checks to the startup scripts to prevent incorrect usage #5977

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
a5dec5f
[#5976] Improvement(bin):Add validation checks to the startup scripts…
liuchunhao Dec 24, 2024
6b38ae9
[#5976] Improvement(bin): Remove the '.template' file extension from …
liuchunhao Dec 24, 2024
fe87ce5
[#5976] Improvement(bin): Update the error message for startup failur…
liuchunhao Dec 25, 2024
73c7c10
[#5976] Improvement(docs): Update getting-started.md
liuchunhao Jan 7, 2025
dbea2d5
[#5976] Improvement(docs): Update getting-started.md & bin/common.sh
liuchunhao Jan 11, 2025
07f02c6
[#5976] Improvement(docs): Update bin/common.sh
liuchunhao Jan 11, 2025
044f464
[#5976] Improvement(docs): Update getting-started.md
liuchunhao Jan 11, 2025
9499434
[#5976] Improvement(docs): Update getting-started.md
liuchunhao Jan 11, 2025
4cfcd6d
[#5976] Improvement(bin): Update the instruction message in common.sh
liuchunhao Jan 11, 2025
3d6c62f
[#5976] Improvement(bin): Update the instruction message in common.sh
liuchunhao Jan 12, 2025
cb8d850
[#5976] Improvement(bin): rename common.sh to common.sh.template
liuchunhao Jan 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions bin/common.sh
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @liuchunhao I think maybe we can modify this file suffix to .sh.template, keep it consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xunliu , thank you for the feedback. I have updated it as suggested.

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@
# Referred from Apache Submarine's common.sh implementation
# bin/common.sh

GRAVITINO_VERSION=GRAVITINO_VERSION_PLACEHOLDER
if [[ "$GRAVITINO_VERSION" == *_VERSION_PLACEHOLDER ]]; then
echo "GRAVITINO_VERSION is not set. Please make sure you are running the script from the distribution/package/bin and before running the script, run './gradle clean build -x test compileDistribution'"
exit 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update the prompt message:

No GRAVITINO_VERSION was found, you may need to:
1. Run the gravitino.sh script on the compiled Gravitino.
2. Run the gravitino.sh script in the release package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback. I will make the updates as required:

  • Update the error message for startup failures
  • Adjust the version check
  • Revert the root build.gradle.kts files

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Run the gravitino.sh script in the release package." this may not be the best instructions, as an ASF release package is source, not binary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does 'release package' mean? Should it instead be referred to as a 'distribution package'?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinmclean The problem is that there are some users who run gravitino.sh scripts without compiling, causing them to fail. We want to give them some tips. Do you have any better suggestions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ask them to use a binary distribution or compile it first? i.e:

  1. Run the gravitino.sh script in a compiled version of Gravitino.
  2. Run the gravitino.sh script from a binary release package.

fi


if [ -L "${BASH_SOURCE-$0}" ]; then
FWDIR=$(dirname "$(readlink "${BASH_SOURCE-$0}")")
else
Expand Down
0 bin/gravitino-iceberg-rest-server.sh → ...gravitino-iceberg-rest-server.sh.template
View file
Open in desktop
File renamed without changes.
0 bin/gravitino.sh → bin/gravitino.sh.template
View file
Open in desktop
File renamed without changes.
4 changes: 2 additions & 2 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ tasks {
fileName.replace(".template", "")
}
eachFile {
if (name == "gravitino-env.sh") {
if (name == "gravitino-env.sh" || name == "common.sh") {
filter { line ->
line.replace("GRAVITINO_VERSION_PLACEHOLDER", "$version")
}
Expand Down Expand Up @@ -643,7 +643,7 @@ tasks {
fileName.replace(".template", "")
}
eachFile {
if (name == "gravitino-env.sh") {
if (name == "gravitino-env.sh" || name == "common.sh") {
filter { line ->
line.replace("GRAVITINO_VERSION_PLACEHOLDER", "$version")
}
Expand Down
Loading