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

Fix wrong ray start command #431

Merged
merged 1 commit into from
Jul 29, 2022
Merged

Fix wrong ray start command #431

merged 1 commit into from
Jul 29, 2022

Conversation

pingsutw
Copy link
Contributor

Signed-off-by: Kevin Su [email protected]

Why are these changes needed?

include-dashboard is not a flag parameter, so we should convert it to --include-dashboard=true instead of --include-dashboard

(base) ➜  ray-operator git:(ping-7) ray start --help                           
Usage: ray start [OPTIONS]

  Start Ray processes manually on the local machine. PublicAPI: This API is
  stable across Ray releases.

Options:
  --node-ip-address TEXT          the IP address of this node
  --address TEXT                  the address to use for Ray
  --port INTEGER                  the port of the head ray process. If not
                                  provided, defaults to 6379; if port is set
                                  to 0, we will allocate an available port.
  --object-manager-port INTEGER   the port to use for starting the object
                                  manager
  --node-manager-port INTEGER     the port to use for starting the node
                                  manager
  --gcs-server-port INTEGER       Port number for the GCS server.
  --min-worker-port INTEGER       the lowest port number that workers will
                                  bind on. If not set, random ports will be
                                  chosen.
  --max-worker-port INTEGER       the highest port number that workers will
                                  bind on. If set, '--min-worker-port' must
                                  also be set.
  --worker-port-list TEXT         a comma-separated list of open ports for
                                  workers to bind on. Overrides '--min-worker-
                                  port' and '--max-worker-port'.
  --ray-client-server-port INTEGER
                                  the port number the ray client server will
                                  bind on. If not set, the ray client server
                                  will not be started.
  --object-store-memory INTEGER   The amount of memory (in bytes) to start the
                                  object store with. By default, this is
                                  capped at 20GB but can be set higher.
  --num-cpus INTEGER              the number of CPUs on this node
  --num-gpus INTEGER              the number of GPUs on this node
  --resources TEXT                a JSON serialized dictionary mapping
                                  resource name to resource quantity
  --head                          provide this argument for the head node
  --include-dashboard BOOLEAN     provide this argument to start the Ray
                                  dashboard GUI
  --dashboard-host TEXT           the host to bind the dashboard server to,
                                  either localhost (127.0.0.1) or 0.0.0.0
                                  (available from all interfaces). By default,
                                  thisis localhost.
  --dashboard-port INTEGER        the port to bind the dashboard server to--
                                  defaults to 8265
  --block                         provide this argument to block forever in
                                  this command
  --plasma-directory TEXT         object store directory for memory mapped
                                  files
  --autoscaling-config TEXT       the file that contains the autoscaling
                                  config
  --no-redirect-output            do not redirect non-worker stdout and stderr
                                  to files
  --plasma-store-socket-name TEXT
                                  manually specify the socket name of the
                                  plasma store
  --raylet-socket-name TEXT       manually specify the socket path of the
                                  raylet process
  --storage TEXT                  the persistent storage URI for the cluster.
                                  Experimental.
  --ray-debugger-external         Make the Ray debugger available externally
                                  to the node. This is onlysafe to activate if
                                  the node is behind a firewall.
  --disable-usage-stats           If True, the usage stats collection will be
                                  disabled.
  --log-style [auto|record|pretty]
                                  If 'pretty', outputs with formatting and
                                  color. If 'record', outputs record-style
                                  without formatting. 'auto' defaults to
                                  'pretty', and disables pretty logging if
                                  stdin is *not* a TTY.
  --log-color [auto|false|true]   Use color logging. Auto enables color
                                  logging if stdout is a TTY.
  -v, --verbose
  --help                          Show this message and exit.

Related issue number

Checks

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

for k, v := range rayStartParams {
if strings.ToLower(v) == "true" {
if strings.ToLower(v) == "true" && !utils.Contains(nonFlagParams, k) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice catch. I think strings.ToLower(v) == "true" is not always correct for such cases. We internally use --dashboard-host directly and didn't notice this issue. The change looks good to me, we may leave an eye on new added flags in the future

@Jeffwan Jeffwan merged commit e0ccff5 into ray-project:master Jul 29, 2022
Jeffwan pushed a commit to Jeffwan/kuberay that referenced this pull request Aug 9, 2022
Jeffwan added a commit that referenced this pull request Aug 10, 2022
* Fix nil pointer dereference (#429)

Signed-off-by: Kevin Su <[email protected]>

* Fix wrong ray start command (#431)

Signed-off-by: Kevin Su <[email protected]>

* Add ray state api doc link in ray service doc (#428)

* Add ray state api doc link in ray service doc

* Update doc

* update

* [doc] Fix config typos

Signed-off-by: Dmitri Gekhtman <[email protected]>

Fixes a couple of typos in recently introduced sample configs.

* Add http resp code check for kuberay (#435)

* Clean up example samples (#434)

This PR cleans up the "complete" and "autoscaler" sample yamls a bit.
Unnecessary pod spec fields are removed without sacrificing the completeness of the examples.
The idea is to make the configuration look less intimidating.

Signed-off-by: Dmitri Gekhtman <[email protected]>

* Add more env for RayService head or worker pods (#439)

* fix: worker node can't connect to head node service (#445)

Signed-off-by: Kevin Su <[email protected]>

* helm-chart/ray-cluster: allow head autoscaling (#443)

Also allow setting rayVersion

Signed-off-by: Christos Kotsis <[email protected]>

* Disable async serve handler in Ray Service cluster (#447)

* Add wget timeout to probes (#448)

* Enable tests against release-0.3 branch

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Dmitri Gekhtman <[email protected]>
Signed-off-by: Christos Kotsis <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Co-authored-by: bruce <[email protected]>
Co-authored-by: Dmitri Gekhtman <[email protected]>
Co-authored-by: Christos Kotsis <[email protected]>
Co-authored-by: Yi Cheng <[email protected]>
Co-authored-by: Wilson Wang <[email protected]>
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