Skip to content

Allow pausing and resuming of the worker#229

Merged
DiamondJoseph merged 2 commits intomainfrom
shutdown
May 25, 2023
Merged

Allow pausing and resuming of the worker#229
DiamondJoseph merged 2 commits intomainfrom
shutdown

Conversation

@DiamondJoseph
Copy link
Copy Markdown
Contributor

No description provided.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 24, 2023

Codecov Report

Merging #229 (060e592) into main (8c97988) will decrease coverage by 0.50%.
The diff coverage is 67.64%.

@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
- Coverage   89.06%   88.57%   -0.50%     
==========================================
  Files          41       41              
  Lines        1271     1339      +68     
==========================================
+ Hits         1132     1186      +54     
- Misses        139      153      +14     
Impacted Files Coverage Δ
src/blueapi/cli/cli.py 72.99% <50.98%> (-12.88%) ⬇️
src/blueapi/cli/amq.py 62.06% <58.82%> (+23.18%) ⬆️
src/blueapi/cli/rest.py 81.81% <83.33%> (-0.69%) ⬇️
src/blueapi/service/handler.py 83.33% <100.00%> (ø)
src/blueapi/service/main.py 82.27% <100.00%> (+3.49%) ⬆️
src/blueapi/service/model.py 90.90% <100.00%> (+0.66%) ⬆️
src/blueapi/worker/reworker.py 94.90% <100.00%> (+0.14%) ⬆️
src/blueapi/worker/worker.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Copy Markdown
Contributor

@tpoliaw tpoliaw left a comment

Choose a reason for hiding this comment

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

It doesn't feel very RESTy to use query parameters for the payload of a PUT request. Should it be a {"status": "pause", "defer": true} body instead?

Comment thread src/blueapi/service/main.py Outdated
Comment thread src/blueapi/service/main.py Outdated
@DiamondJoseph DiamondJoseph changed the title Allow shutdown, resume, pause of the run engine Allow pausing and resuming of the worker May 25, 2023
Comment thread src/blueapi/service/main.py Outdated
Comment thread tests/service/test_rest_api.py Outdated
Comment thread tests/service/test_rest_api.py Outdated
@DiamondJoseph DiamondJoseph marked this pull request as ready for review May 25, 2023 09:10
'404':
description: Not Found
item: not found
detail: item not found
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the same as the 404 message returned when a KeyError or ValueError is thrown. Using it as a standard will probably save us time later on.

@DiamondJoseph DiamondJoseph requested a review from keithralphs May 25, 2023 09:13
Comment thread src/blueapi/service/main.py
Comment thread tests/conftest.py


@pytest.fixture(scope="session")
@pytest.fixture
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

scope="session" left state hanging around, its inclusion was due to a misunderstanding of fixtures.

Comment thread src/blueapi/service/main.py
Copy link
Copy Markdown
Contributor

@rosesyrett rosesyrett left a comment

Choose a reason for hiding this comment

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

just a few questions/comments

Comment thread src/blueapi/cli/rest.py Outdated
Comment thread src/blueapi/cli/rest.py Outdated
Copy link
Copy Markdown
Contributor

@rosesyrett rosesyrett left a comment

Choose a reason for hiding this comment

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

ty

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.

Endpoint for aborting/pausing/resuming the currently running task

6 participants