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

Add a method to stop a running Engine #1355

Merged
merged 1 commit into from
Mar 12, 2020
Merged

Conversation

hynd
Copy link
Contributor

@hynd hynd commented Mar 10, 2020

Including a new stop cmd to trigger it via the HTTP API.
Closes #1352.

@CLAassistant
Copy link

CLAassistant commented Mar 10, 2020

CLA assistant check
All committers have signed the CLA.

cmd/stop.go Outdated Show resolved Hide resolved
cmd/stop.go Outdated Show resolved Hide resolved
core/engine.go Show resolved Hide resolved
core/engine.go Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Mar 10, 2020

Codecov Report

Merging #1355 into new-schedulers will decrease coverage by 0.00%.
The diff coverage is 36.84%.

Impacted file tree graph

@@                Coverage Diff                 @@
##           new-schedulers    #1355      +/-   ##
==================================================
- Coverage           76.60%   76.59%   -0.01%     
==================================================
  Files                 160      160              
  Lines               12567    12584      +17     
==================================================
+ Hits                 9627     9639      +12     
- Misses               2431     2436       +5     
  Partials              509      509              
Impacted Files Coverage Δ
api/v1/status_routes.go 10.20% <0.00%> (-0.67%) ⬇️
core/engine.go 91.45% <76.47%> (-1.56%) ⬇️
api/v1/status.go 100.00% <100.00%> (ø)
lib/helpers.go 100.00% <0.00%> (+4.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 567e5a0...bf4c124. Read the comment docs.

@na--
Copy link
Member

na-- commented Mar 11, 2020

@hynd, to fix these linter issues locally, take a look at the "Running the linter" section in https://github.com/loadimpact/k6/blob/master/CONTRIBUTING.md#development-setup

Also, I realize that a k6 stop command would be convenient, but I'm not quite convinced by its necessity. I don't mind if we add a new minor REST API method, but a new-subcommand requires some more consideration...

@hynd hynd force-pushed the stop_api branch 2 times, most recently from 31afae9 to 439788c Compare March 11, 2020 21:28
@hynd
Copy link
Contributor Author

hynd commented Mar 11, 2020

Understood - the stop command wasn't something i needed, just seemed quick to implement :)
I've removed it and fixed up the CI items.

Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

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

LGTM

@hynd hynd force-pushed the stop_api branch 2 times, most recently from df03d0e to 15d1edd Compare March 11, 2020 22:57
core/engine_test.go Outdated Show resolved Hide resolved
@hynd hynd marked this pull request as ready for review March 12, 2020 10:11
@na-- na-- merged commit 4017d71 into grafana:new-schedulers Mar 12, 2020
@na--
Copy link
Member

na-- commented Mar 12, 2020

Thanks!

@hynd hynd deleted the stop_api branch December 3, 2020 13:00
@na-- na-- added this to the v0.27.0 milestone Jan 19, 2021
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.

6 participants