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.

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

Install: Fix directory path for prometheus install.sh #256

Merged
merged 1 commit into from
May 12, 2022

Conversation

Tomcli
Copy link
Contributor

@Tomcli Tomcli commented May 11, 2022

Why are these changes needed?

Right now the /install/prometheus/install.sh script is assuming the directory path is /install/prometheus/. However, the main README instruction is asking users to run this script from the top directory which will result the following errors:

+ monitor_dir=../../config/prometheus
+ pushd ../../config/prometheus
./install/prometheus/install.sh: line 14: pushd: ../../config/prometheus: No such file or directory

Therefore, we need to make sure monitor_dir is set to the right directory without depending on the user's current directory path.

Related issue number

None

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@Tomcli Tomcli changed the title Fix directory path for prometheus install.sh Install: Fix directory path for prometheus install.sh May 11, 2022
@Jeffwan
Copy link
Collaborator

Jeffwan commented May 12, 2022

This is a nice catch and thanks for the fix. Prometheus support is recently added and community may not have enough testing on user guide. Really appreciate your findings!

@Jeffwan Jeffwan merged commit c7356da into ray-project:master May 12, 2022
daikeshi pushed a commit to daikeshi/kuberay that referenced this pull request Jul 8, 2022
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants