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

Jump to bottom

[Improvement] Add validation checks to the startup scripts to prevent incorrect usage #5976

Closed
liuchunhao opened this issue Dec 24, 2024 · 1 comment
Closed

[Improvement] Add validation checks to the startup scripts to prevent incorrect usage #5976

liuchunhao opened this issue Dec 24, 2024 · 1 comment
Assignees
@liuchunhao
Labels
0.9.0 Release v0.9.0 improvement Improvements on everything

Comments

@liuchunhao
Copy link
Contributor

What would you like to be improved?

For users who are new to Gravitino, many attempt to launch Gravitino services using the script located at /gravitino/bin, such as /gravitino/bin/gravitino.sh, which is only a script template.

The correct way is to run ./gradlew clean build -x test compileDistribution, which will compile and generate all artifacts into the directory /gravitino/distribution/package, including /gravitino/distribution/package/bin/gravitino.sh.
Then you can use /gravitino/distribution/package/bin/gravitino.sh to launch the services.

How should we improve?

To avoid the above misuse, we have a few suggestions to prevent mistakes:

  1. Add a .template extension to the scripts bin/gravitino.sh, gravitino-iceberg-rest-server.sh, and common.sh to ensure they cannot be run by accident.
  2. Add a check for the GRAVITINO_VERSION environment variable in the gravitino.sh.template and gravitino-iceberg-rest-server.sh.template scripts. If GRAVITINO_VERSION is not assigned, print an error message and exit the script.
  3. Ensure that only the scripts in the distribution/package/bin/ directory have the GRAVITINO_VERSION environment variable set.
@liuchunhao liuchunhao added the improvement Improvements on everything label Dec 24, 2024
@liuchunhao
Copy link
Contributor Author

I would like to work on this

liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Dec 24, 2024
@liuchunhao
[apache#5976] Improvement(bin):Add validation checks to the startup s…
a5dec5f 
…cripts to prevent incorrect usage
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Dec 24, 2024
@liuchunhao
[apache#5976] Improvement(bin): Remove the '.template' file extension…
6b38ae9 
… from bin/common.sh
@xunliu xunliu mentioned this issue Dec 25, 2024
Merged
@xunliu xunliu added the 0.8.0 Release v0.8.0 label Dec 25, 2024
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Dec 25, 2024
@liuchunhao
[apache#5976] Improvement(bin): Update the error message for startup …
fe87ce5 
…failures /Adjust the version check /Revert the root build.gradle.kts files
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 7, 2025
@liuchunhao
[apache#5976] Improvement(docs): Update getting-started.md
73c7c10
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 11, 2025
@liuchunhao
[apache#5976] Improvement(docs): Update getting-started.md & bin/comm…
dbea2d5 
…on.sh
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 11, 2025
@liuchunhao
[apache#5976] Improvement(docs): Update bin/common.sh
07f02c6
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 11, 2025
@liuchunhao
[apache#5976] Improvement(docs): Update getting-started.md
044f464
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 11, 2025
@liuchunhao
[apache#5976] Improvement(docs): Update getting-started.md
9499434
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 11, 2025
@liuchunhao
[apache#5976] Improvement(bin): Update the instruction message in com…
4cfcd6d 
…mon.sh
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 12, 2025
@liuchunhao
[apache#5976] Improvement(bin): Update the instruction message in com…
3d6c62f 
…mon.sh
liuchunhao added a commit to liuchunhao/gravitino that referenced this issue Jan 13, 2025
@liuchunhao
[apache#5976] Improvement(bin): rename common.sh to common.sh.template
cb8d850
@jerryshao jerryshao removed the 0.8.0 Release v0.8.0 label Jan 13, 2025
jerryshao pushed a commit that referenced this issue Jan 15, 2025
@liuchunhao
[#5976] Improvement(bin):Add validation checks to the startup scripts…
39ad18a 
… to prevent incorrect usage (#5977)

### What changes were proposed in this pull request?

#5976

- Add file suffix ‘template’ to the following scripts:
    - bin/gravitino.sh
    - bin/common.sh
    - bin/gravitino-iceberg-rest-server.sh
- Add a validation check on `GRAVITINO_VERSION` in the script
bin/common.sh ( renamed to bin/common.sh.template ) with the followings
:
    
    ```bash
    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
    fi
    
    ```
    
- Update the following tasks in the root build.gradle.kts as described
below :
    - compileDistribution
    - compileIcebergRESTServer
    ```bash
     eachFile {
          if (name == "gravitino-env.sh" || name == "common.sh") {
            filter { line ->
              line.replace("GRAVITINO_VERSION_PLACEHOLDER", "$version")
            }
          }
        }
    ```

### Why are the changes needed?

To prevent incorrect usage with startup scripts

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?
- The scripts below will exit with status 1 and print an error message
with the correct instructions
```bash

cd bin
gravitino.sh.template start    
gravitino-iceberg-rest-server.sh.template start
```

- correct way to run gravitino : 
```bash
./gradle clean build -x test compileDistribution

cd distribution/package/bin

./gravitino.sh start

./gravitino-iceberg-rest-server.sh start

```
@jerryshao jerryshao added the 0.9.0 Release v0.9.0 label Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@liuchunhao liuchunhao

Labels
0.9.0 Release v0.9.0 improvement Improvements on everything
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@jerryshao @xunliu @liuchunhao