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

[Bug] "Restart if recording already exists" checkbox doesn't work #1670

Closed
andrewazores opened this issue Sep 15, 2023 · 5 comments · Fixed by #1672
Closed

[Bug] "Restart if recording already exists" checkbox doesn't work #1670

andrewazores opened this issue Sep 15, 2023 · 5 comments · Fixed by #1672
Assignees
Labels
bug Something isn't working

Comments

@andrewazores
Copy link
Member

Current Behavior

Creating a recording with name Foo, then trying to create another recording with name Foo, does fails with an HTTP 400.

Expected Behavior

The request should fail if the "restart" checkbox is not checked, but should pass and result in the recording being restarted if the box is checked.

Steps To Reproduce

  1. Run sh smoketest.sh
  2. Go to web UI
  3. Go to Recordings
  4. Select a target, etc. vertx-fib-demo-1
  5. Create a recording manually, ex. named test, continuous duration, Continuous template
  6. Try to repeat step 5 with and without the "restart" checkbox checked

Environment

No response

Anything else?

image

@andrewazores andrewazores added the bug Something isn't working label Sep 15, 2023
@andrewazores andrewazores moved this to Todo in 2.4.0 release Sep 15, 2023
@tthvo
Copy link
Member

tthvo commented Sep 15, 2023

@andrewazores
Copy link
Member Author

Right -

https://github.com/cryostatio/cryostat/pull/1587

https://github.com/cryostatio/cryostat/pull/1587/files#diff-52b7be8bc2188ea065259592ef8710555d3d73dabd58cd2ce0b3a57ab4c6f999L155

I recall we had some discussion around this change before but I can't remember exactly what we landed on. Since this is changing in a minor release version the API "shouldn't" change in incompatible ways, but I'm okay with changing a query parameter like this. Or we could have the endpoint handler handle both forms and prefer the newer one if both are supplied.

@andrewazores
Copy link
Member Author

andrewazores commented Sep 15, 2023

ie.

ReplacementPolicy replace = ReplacementPolicy.NEVER;
if (attrs.contains("restart")) {
  replace = Boolean.parseBoolean(attrs.get("restart")) ? ReplacementPolicy.ALWAYS : ReplacementPolicy.NEVER;
}
if (attrs.contains("replace")) {
  replace = ReplacementPolicy.fromString(attrs.get("replace"));
}

@tthvo
Copy link
Member

tthvo commented Sep 15, 2023

This one looks good! I can help apply above fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants