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.

Sign up for GitHub

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

Jump to bottom

feat(rpc/gateway): move to ListenAndServe #1286

Closed
Tracked by #944
distractedm1nd opened this issue Oct 28, 2022 · 1 comment · Fixed by #1314
Closed
Tracked by #944

feat(rpc/gateway): move to ListenAndServe #1286

distractedm1nd opened this issue Oct 28, 2022 · 1 comment · Fixed by #1314
Assignees
@distractedm1nd
Labels
area:rpc

Comments

@distractedm1nd
Copy link
Collaborator

distractedm1nd commented Oct 28, 2022 •
edited
Loading

We can use ListenAndServe, and get rid of the listener on the Server structs in gateway+rpc by creating a new constructor that overrides the http server. Using ListenAndServe alone does not allow us to use port 0 in tests without losing the port that gets assigned. @walldiss pointed out we can create a new constructor and pass in a server from the httptest package.

    Update the server to not use Listener and make lifecycling over HTTP Server directly

Originally posted by @Wondertan in #1199 (comment)
@distractedm1nd distractedm1nd mentioned this issue Oct 28, 2022
Merged
This was referenced Oct 28, 2022
Closed
Merged
Merged
@distractedm1nd distractedm1nd changed the title feat(rpc/gateway): add WithServer to servers + move to ListenAndServe feat(rpc/gateway): move to ListenAndServe Nov 2, 2022
@distractedm1nd distractedm1nd moved this to TODO in Celestia Node Nov 9, 2022
@distractedm1nd distractedm1nd moved this from TODO to In Review in Celestia Node Nov 9, 2022
@distractedm1nd distractedm1nd self-assigned this Nov 9, 2022
@distractedm1nd
Copy link
Collaborator Author

Closing in favor of #1322

Repository owner moved this from In Review to Done in Celestia Node Nov 9, 2022
distractedm1nd pushed a commit that referenced this issue Nov 9, 2022
fix(gateway): switch to separate Listen/Serve (#1314)
f7026ff 
Co-authored-by: rene <41963722+renaynay@users.noreply.github.com>
Closes #1286
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@distractedm1nd distractedm1nd

Labels
area:rpc
Projects
No open projects
Celestia Node
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(gateway): switch to separate Listen/Serve distractedm1nd/celestia-node
1 participant
@distractedm1nd