Skip to content

[rayapp] switch workspace ops from --name to --id#491

Closed
elliot-barn wants to merge 1 commit into
mainfrom
elliot-barn-workspace-by-id
Closed

[rayapp] switch workspace ops from --name to --id#491
elliot-barn wants to merge 1 commit into
mainfrom
elliot-barn-workspace-by-id

Conversation

@elliot-barn

Copy link
Copy Markdown
Collaborator

Why

The Anyscale API endpoint backing anyscale workspace_v2 <op> --name <name>
(GET /api/v2/experimental_workspaces/) was observed returning HTTP 404 on
console.anyscale-staging.com. The by-id endpoint
(GET /api/v2/experimental_workspaces/{workspace_id}) is unaffected.

Because the CLI swallows the 404 and reports it as
Workspace with name '<name>' was not found., rayapp test fails
immediately after creating a workspace — the next step is
workspace_v2 get --name ..., which can never succeed even though the
workspace exists. The misleading error was reproduced on anyscale CLI
versions 0.26.87, 0.26.95, 0.26.99, and 0.26.100, confirming the regression
is server-side, not in the CLI.

What changed

rayapp now captures the workspace id from workspace_v2 create stdout/stderr
(Workspace created successfully id: expwrk_...) and passes --id <id> to
every subsequent workspace_v2 subcommand. The by-name code path is removed.

Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the AnyscaleCLI wrapper to address workspaces by their unique ID instead of their name, which resolves reliability issues observed with the Anyscale API. It introduces a new method to run the CLI with combined stdout/stderr output to parse the newly created workspace ID via a regular expression, and updates all workspace subcommands to use the --id flag. Feedback suggests making the workspace ID regular expression more robust by removing the hardcoded expwrk_ prefix, and improving error reporting when parsing fails by including the workspace name and the actual CLI output in the error message.

Comment on lines +35 to +37
var workspaceCreatedIDRe = regexp.MustCompile(
`Workspace created successfully id:\s+(expwrk_[A-Za-z0-9_-]+)`,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Hardcoding the expwrk_ prefix in the regular expression makes it fragile. If Anyscale graduates workspaces from experimental or changes the workspace ID prefix in the future (e.g., to wrk_ or ws_), this regex will fail to match, causing all workspace creations to fail parsing and potentially leak resources.

Consider making the regex more robust by matching any valid ID format without hardcoding the prefix.

Suggested change
var workspaceCreatedIDRe = regexp.MustCompile(
`Workspace created successfully id:\s+(expwrk_[A-Za-z0-9_-]+)`,
)
var workspaceCreatedIDRe = regexp.MustCompile(
`Workspace created successfully id:\s+([A-Za-z0-9_-]+)`,
)

Comment on lines +87 to 91
if len(m) != 2 {
return "", fmt.Errorf(
"create empty workspace: could not parse workspace id from output",
)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

If the regex fails to match the workspace ID, the error message does not include the actual CLI output or the workspace name. This makes troubleshooting extremely difficult if Anyscale changes its output format, and it also makes it hard to identify which workspace was leaked for manual cleanup.

Including the workspace name and the actual output in the error message will greatly improve observability and debuggability.

Suggested change
if len(m) != 2 {
return "", fmt.Errorf(
"create empty workspace: could not parse workspace id from output",
)
}
if len(m) != 2 {
return "", fmt.Errorf(
"create empty workspace %q: could not parse workspace id from output: %q",
c.workspaceName, out,
)
}

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.

1 participant