apache / gravitino-playground

A playground to experience Gravitino
Apache License 2.0
31 stars 31 forks source link

[#102] improvement: improve startup script #103

Open orenccl opened 2 weeks ago

orenccl commented 2 weeks ago

What changes were proposed in this pull request?

  1. Removed the debug option set -x.
  2. Fixed warnings identified by IntelliJ, such as changing cd "${playground_dir}" >/dev/null to cd "${playground_dir}" >/dev/null || exit 1 to prevent failures when cd fails.
  3. Updated docker run --pull always hello-world >/dev/null 2>&1 to docker run --rm --pull always hello-world:latest >/dev/null 2>&1 to avoid pulling multiple images and ensure the container is removed after it finishes.
  4. Changed naming conventions and log formats for consistency.
  5. Added disk, RAM, and CPU checks.
  6. Added the --skip-checks option for a quicker startup.
  7. Corrected the log suffix format from %Y%m%d%H%m%s to %Y%m%d%H%M%s.
  8. Removed the unnecessary Confirm the requirement is available in your OS [Y/n]:.
  9. Combined port check status into one line instead of multiple lines.

Why are the changes needed?

To improve the user experience. close #102

Does this PR introduce any user-facing change?

Added a new option --skip-checks|-s for a quicker startup.

How was this patch tested?

Tested with the following commands:

These tests were run to ensure the script works as intended.

image

orenccl commented 2 weeks ago

Hi @unknowntpo,

I've resolved the conflicts based on your recently merged PR #56.

Would you please review this? If possible, I would appreciate if you could also test it in your environment.

Thanks!

image

unknowntpo commented 2 weeks ago

@orenccl Okay, I'll take a look :)

xunliu commented 2 weeks ago

hi @orenccl Thank you for your improve playground I think we didn't neet specify docker|k8s param, we can check use environment,

  1. if user only have docker, then launch playground on Docker
  2. if use only have k8s, then launch playground on Docker
  3. if use both have docker and k8s, then output a menu, let use select one to launch.
orenccl commented 1 week ago

image

image

image

unknowntpo commented 1 week ago
  • if user only have docker, then launch playground on Docker

  • if use only have k8s, then launch playground on Docker

  • if use both have docker and k8s, then output a menu, let use select one to launch.

@xunliu I think letting user to run both k8s and docker is more handy.

orenccl commented 1 week ago

Update this PR:

  1. Unified port number for Docker and Kubernetes.
  2. update readme

Please let me know if there are any problems.

jerryshao commented 4 days ago

Hi @orenccl would you please rebase this PR to fix the conflicts?

danhuawang commented 4 days ago

Update this PR:

  1. Unified port number for Docker and Kubernetes.
  2. update readme

Please let me know if there are any problems.

@orenccl Can you help check this issue when I ran the fileset example in jupyter? By the way ,we have 4 examples in jupyter, can you help verify them by access http://localhost:18888 ,especially k8s runtime.

image
orenccl commented 3 days ago

Hi @danhuawang,

Sorry about this. I found that the commit Unified port number for Docker and Kubernetes actually caused some issues with internal communication within the Kubernetes cluster.

This should be addressed in a separate issue, so I have reverted it and focused on the original problem. I've also updated the README to clarify the container and host port mapping, so users won't be confused about which port to forward.

Could you review this when you have free time?

image

danhuawang commented 2 days ago

@orenccl conflicts need to be resolved.

danhuawang commented 1 day ago

@qqqttt123 Any comments?