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 CLI commands to inspect/restart the environment #366

Closed
joeshannon opened this issue Feb 5, 2024 · 12 comments · Fixed by #409
Closed

Add CLI commands to inspect/restart the environment #366

joeshannon opened this issue Feb 5, 2024 · 12 comments · Fixed by #409
Assignees
Labels
cli Relates to CLI code client Relates to client code good first issue Good for newcomers

Comments

@joeshannon
Copy link
Contributor

No description provided.

@joeshannon joeshannon added good first issue Good for newcomers cli Relates to CLI code labels Feb 5, 2024
@callumforrester callumforrester added the client Relates to client code label Feb 6, 2024
@stan-dot
Copy link
Contributor

stan-dot commented Mar 8, 2024

what are the features of environment we want to inspect here? is it about pip environment? or beamline-specific config?

@stan-dot stan-dot self-assigned this Mar 8, 2024
@joeshannon
Copy link
Contributor Author

This to add cli support for the environment end points. This allows the bluesky context to be reloaded without restarting the whole service which is useful if blueapi is loading plans from a "scratch" area to allow rapid development/testing.

@stan-dot
Copy link
Contributor

connected to #363 (?)

looking into the environment now

@stan-dot
Copy link
Contributor

`
@app.get("/environment", response_model=EnvironmentResponse)
def get_environment(
handler: BlueskyHandler = Depends(get_handler),
) -> EnvironmentResponse:
return EnvironmentResponse(initialized=handler.initialized)

@app.delete("/environment")
async def delete_environment(
background_tasks: BackgroundTasks,
handler: BlueskyHandler = Depends(get_handler),
):
def restart_handler(handler: BlueskyHandler):
handler.stop()
handler.start()

if handler.initialized:
    background_tasks.add_task(restart_handler, handler)

`

I see only two endpoints right now - get and delete, no 'reload'. Reading into the calls from delete it looks like that it is how the reload should be performed.

would the minimal implementation user experience be of this kind: blueapi env and blueapi env reload? with some alternatives names possible ('env r') for faster reload?

@callumforrester
Copy link
Contributor

You're correct, "reload" is a verb, so shouldn't be in the rest api. But the CLI can combine nouns into a verb. So your suggested UX looks good.

@stan-dot
Copy link
Contributor

ok will implement that, it should be a pretty minimal addition

@stan-dot
Copy link
Contributor

not sure if 'environment' is the best name here

one confusing thing is the similarity between 'infrastructure environment', 'environment variables' and what we have here is 'devices ensemble' or 'devices config'. [just brainstorming]

@stan-dot
Copy link
Contributor

currently {'initialized': True} is as much of an inspection main.py allows:

@app.get("/environment", response_model=EnvironmentResponse) def get_environment( handler: BlueskyHandler = Depends(get_handler), ) -> EnvironmentResponse: return EnvironmentResponse(initialized=handler.initialized)

@stan-dot
Copy link
Contributor

using blueapi controller env as that click command group has already the rest client in the context, hence as this env command also uses the rest client it only makes sense that it is under command not main

@stan-dot
Copy link
Contributor

none is not an allowed value (type=type_error.none.not_allowed) Pydantic complains. @callumforrester could you check cli.py:114?

@stan-dot stan-dot linked a pull request Apr 2, 2024 that will close this issue
@stan-dot
Copy link
Contributor

stan-dot commented Apr 3, 2024

thinking how to fix the test which fails now that there's the 'sleep(5) line there

@stan-dot
Copy link
Contributor

stan-dot commented Apr 24, 2024

testing sleep does not seem to be well-supported
pytest-dev/pytest#11196

most use cases are to use 'sleep' in the test body itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Relates to CLI code client Relates to client code good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants