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

Cleanup: remove server partial move of self in run #112

Open
kriswuollett opened this issue May 11, 2023 · 0 comments · May be fixed by #113 or #111
Open

Cleanup: remove server partial move of self in run #112

kriswuollett opened this issue May 11, 2023 · 0 comments · May be fixed by #113 or #111

Comments

@kriswuollett
Copy link
Contributor

It may make more sense to split run into start and join and merge bind into start in order to have a clean way to give locally bound ports to tests from a start method which also helps guarantee in a way that the web server has started serving on the bound port with a listener before tests hit it.

kriswuollett added a commit to kriswuollett/registry that referenced this issue May 12, 2023
The impetus behind the change is to support adding monitoring endpoints with
an optional additional TCP listener. While refactoring the server it was
discovered that the run method moves the Server self, and so fixed bytecodealliance#112 to
also ensure that the API endpoint was running before tests could discover the
listener's locally bound port.

In addition a common cancellation token is added to the server to propagate
shutdown to both the core service as well as the API server endpoint. It can be
later used to support additional services later such as a monitoring
endpoint(s).
@kriswuollett kriswuollett linked a pull request May 12, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant