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

Break HTTP routes into multiple servers #1087

Merged
merged 12 commits into from
Aug 18, 2023
Merged

Conversation

joshklop
Copy link
Contributor

For access control purposes.

@joshklop joshklop requested a review from omerfirmak August 15, 2023 20:40
@joshklop
Copy link
Contributor Author

This is probably easiest to review all at once, rather than commit-by-commit. We can squash merge.

@joshklop joshklop force-pushed the joshklop/http-breakup branch from 518db45 to 757f3f5 Compare August 15, 2023 20:41
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Patch coverage: 91.78% and project coverage change: +0.06% 🎉

Comparison is base (b0bfb09) 73.81% compared to head (6cca12a) 73.87%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1087      +/-   ##
==========================================
+ Coverage   73.81%   73.87%   +0.06%     
==========================================
  Files          66       66              
  Lines        7255     7270      +15     
==========================================
+ Hits         5355     5371      +16     
- Misses       1449     1453       +4     
+ Partials      451      446       -5     
Files Changed Coverage Δ
node/node.go 46.61% <88.46%> (+15.58%) ⬆️
node/http.go 92.26% <92.50%> (ø)
cmd/juno/juno.go 57.31% <100.00%> (+3.98%) ⬆️

... and 4 files with indirect coverage changes

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

@omerfirmak
Copy link
Contributor

We can put Http & ws rpc on the same port, WDYT?

@joshklop joshklop force-pushed the joshklop/http-breakup branch from 3783be4 to b7b46d0 Compare August 15, 2023 22:14
@joshklop
Copy link
Contributor Author

I'll figure out why the codecov is low and fix the linting errors. I may also change default port numbers to align with what is in the current released version to maximize compatibility.

@joshklop joshklop linked an issue Aug 16, 2023 that may be closed by this pull request
@joshklop joshklop force-pushed the joshklop/http-breakup branch 2 times, most recently from a29c358 to 8e54844 Compare August 16, 2023 19:21
@joshklop
Copy link
Contributor Author

joshklop commented Aug 16, 2023

Rebased and made the adjustments mentioned in my last message. If you don't mind taking another quick look @omerfirmak, I would really appreciate it. I'm fine with merging the http and websocket servers if you think that's a good idea. I kept them separate to follow geth's lead. Let me know if you think that's worth the effort.

Note that all services that require ports are disabled by default now. Enabling them all on their default ports would look something like:

juno --http --ws --metrics --pprof --grpc

@omerfirmak omerfirmak force-pushed the joshklop/http-breakup branch 2 times, most recently from 52de516 to 30faf73 Compare August 17, 2023 08:43
@omerfirmak omerfirmak force-pushed the joshklop/http-breakup branch from 623c678 to b9ac10b Compare August 17, 2023 12:46
@joshklop joshklop force-pushed the joshklop/http-breakup branch from 8fb82d0 to 6cca12a Compare August 18, 2023 02:42
@omerfirmak omerfirmak merged commit 11d2ec3 into main Aug 18, 2023
@omerfirmak omerfirmak deleted the joshklop/http-breakup branch August 18, 2023 06:48
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.

Split HTTP routes into separate servers
2 participants