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

Provide api call to Stop experiment #57

Merged
merged 10 commits into from
Jun 17, 2022
Merged

Conversation

johnaohara
Copy link
Contributor

There are use cases that require stopping an already running experiment. This PR provides REST & gRPC endpoints to allow users to stop a running experiment.
The PR includes sanity checks for remove removing running tests.

One commit that is not directly related to the main functionality of the PR is some changes to speed up running the test suite. Running the full testsuite went from 2562 seconds to 107 seconds on my machine, and allowed me to find a couple of race/locking issues. If it is deemed appropriate to separate out that change, I can revert that commit in this PR and open a separate PR

@johnaohara johnaohara mentioned this pull request Jun 13, 2022
@dinogun
Copy link
Contributor

dinogun commented Jun 14, 2022

@johnaohara Thank you for adding this, should make it easier for scripts to integrate HPO into workflows thanks to this feature. Just one comment, can you update the API doc for the new API

@johnaohara
Copy link
Contributor Author

@dinogun np, I will update the API doc

Signed-off-by: John OHara <johara@redhat.com>
@johnaohara
Copy link
Contributor Author

@dinogun I have updated the API Doc, and modified the response code to be consistent with the other POST commands. Quick question, is there a reason that the rest api's return -1 with a http 4xx response, and not return a message that is meaningful for the client?

…r issues for tests to work with stop experiment
@dinogun
Copy link
Contributor

dinogun commented Jun 16, 2022

@dinogun I have updated the API Doc, and modified the response code to be consistent with the other POST commands. Quick question, is there a reason that the rest api's return -1 with a http 4xx response, and not return a message that is meaningful for the client?

@johnaohara Yes, that was convenient for us when HPO was part of Autotune and returning specific error codes. This needs to be changed going forward.

@dinogun
Copy link
Contributor

dinogun commented Jun 16, 2022

@chandrams Are you done with your test changes?

@chandrams
Copy link
Contributor

@dino - Yes, I 'm done with the test changes.

Copy link
Contributor

@dinogun dinogun left a comment

Choose a reason for hiding this comment

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

LGTM

@dinogun dinogun merged commit ff63102 into kruize:main Jun 17, 2022
@johnaohara johnaohara deleted the stop_experiment branch June 21, 2022 09:28
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.

3 participants