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

366 add cli commands to inspectrestart the environment #409

Merged
merged 39 commits into from
Jun 20, 2024

Conversation

stan-dot
Copy link
Contributor

@stan-dot stan-dot commented Apr 2, 2024

No description provided.

@stan-dot stan-dot linked an issue Apr 2, 2024 that may be closed by this pull request
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.06%. Comparing base (31c94c5) to head (665e396).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #409      +/-   ##
==========================================
+ Coverage   90.71%   91.06%   +0.34%     
==========================================
  Files          40       40              
  Lines        1745     1779      +34     
==========================================
+ Hits         1583     1620      +37     
+ Misses        162      159       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/blueapi/cli/rest.py Outdated Show resolved Hide resolved
src/blueapi/cli/cli.py Outdated Show resolved Hide resolved
src/blueapi/cli/cli.py Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
@stan-dot stan-dot force-pushed the 366-add-cli-commands-to-inspectrestart-the-environment branch from ba583c4 to d790fe1 Compare April 3, 2024 14:46
@stan-dot stan-dot self-assigned this Apr 22, 2024
@stan-dot
Copy link
Contributor Author

src/blueapi/service/main.py:145:22: B008 Do not perform function call Task in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
tests/worker/test_reworker.py:349:18: B008 Do not perform function call Task in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable

not sure what convention do we want to settle on here @callumforrester

tests/worker/test_reworker.py Outdated Show resolved Hide resolved
src/blueapi/service/main.py Outdated Show resolved Hide resolved
.vscode/launch.json Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
src/blueapi/cli/rest.py Outdated Show resolved Hide resolved
src/blueapi/service/main.py Outdated Show resolved Hide resolved
@stan-dot
Copy link
Contributor Author

stan-dot commented May 7, 2024

But the handle error must be in the cli. Otherwise you either quit the program or return something

Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

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

@stan-dot nearly there! Just a couple more things and a rebase

.vscode/launch.json Outdated Show resolved Hide resolved
.vscode/settings.json Outdated Show resolved Hide resolved
Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

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

Happy to approve once the vscode config diff goes away and a rebase is done

@stan-dot
Copy link
Contributor Author

rebase is complex

@stan-dot
Copy link
Contributor Author

will do hectic rebase tomorrow

Copy link
Contributor

@callumforrester callumforrester left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!
(once conflicts are resolved)

@stan-dot
Copy link
Contributor Author

this if failing due to testing coverage. to mock the polling right, we'd need to mock the devices being restarted. the polling logic is simple and custom, not sure what should happen here

@stan-dot stan-dot force-pushed the 366-add-cli-commands-to-inspectrestart-the-environment branch from 4fde54c to 836baec Compare May 23, 2024 13:07
@stan-dot
Copy link
Contributor Author

the CLI demands the access to RabbitMQ. should we spin one up as part of a docker-compose config with devcontainers?

@callumforrester
Copy link
Contributor

@stan-dot it is already spun up, see how the stomp tests are configured

@stan-dot stan-dot force-pushed the 366-add-cli-commands-to-inspectrestart-the-environment branch from 3fcddf6 to 1c2903e Compare May 28, 2024 07:04
@stan-dot stan-dot force-pushed the 366-add-cli-commands-to-inspectrestart-the-environment branch from cdf1fae to 94a7b05 Compare June 3, 2024 09:19
@stan-dot stan-dot requested a review from callumforrester June 3, 2024 10:43
@callumforrester callumforrester force-pushed the 366-add-cli-commands-to-inspectrestart-the-environment branch from 1f7e201 to d0d9694 Compare June 12, 2024 09:28
.vscode/launch.json Outdated Show resolved Hide resolved
tests/worker/test_reworker.py Show resolved Hide resolved
tests/test_cli.py Show resolved Hide resolved
tests/test_cli.py Show resolved Hide resolved
@stan-dot stan-dot merged commit 6a9efe1 into main Jun 20, 2024
24 checks passed
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.

Add CLI commands to inspect/restart the environment
4 participants