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

Environment reload does not block #742

Open
callumforrester opened this issue Dec 2, 2024 · 2 comments · May be fixed by #743
Open

Environment reload does not block #742

callumforrester opened this issue Dec 2, 2024 · 2 comments · May be fixed by #743
Labels
bug Something isn't working cli Relates to CLI code client Relates to client code good first issue Good for newcomers

Comments

@callumforrester
Copy link
Contributor

Reloading the environment via the CLI should block until reload is complete, but does not.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Run the server (blueapi serve)
  2. In separate terminal run blueapi controller env -r, keep both terminals visible at the same time!
  3. Observe the reload command return immediately while the server logs show it taking a couple of seconds to reload.

Acceptance Criteria

  • blueapi env -r blocks until the server reloads
  • If a timeout is supplied it blocks until the timeout is reached
@callumforrester callumforrester added bug Something isn't working client Relates to client code cli Relates to CLI code labels Dec 2, 2024
@callumforrester
Copy link
Contributor Author

This appears to be because the client polls the server to see when the environment is initialized, but actually receives information about the previous environment before it is taken down. The classic fix is to give each environment a unique ID, packaged into EnvironmentResponse so the client can tell the difference between the old and new environments.

@callumforrester callumforrester added the good first issue Good for newcomers label Dec 2, 2024
@callumforrester callumforrester linked a pull request Dec 2, 2024 that will close this issue
@callumforrester
Copy link
Contributor Author

Failing test here: #743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 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.

1 participant