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

feat: go from websocket-server to json rpc websocket/http server #405

Merged
merged 9 commits into from
Nov 6, 2023

Conversation

zeeshanlakhani
Copy link
Contributor

@zeeshanlakhani zeeshanlakhani commented Oct 31, 2023

Includes:

  • re-purposing of feature flags
    • metrics is always a thing (on)
    • monitoring is the gated feature
    • The websocket-server flag is gone, we only gate push notifications
  • JSON-RPC setup and RPC method register
  • Prometheus exposition format to JSON parser
  • Added todo around split networking config

Features & fixes left to add:

  • fix-up example app to use RPC client (in JS/TS)
  • jsonschema / OpenRPC doc generation
  • (maybe) jsonschema validation
  • e2e testing of run workflow
  • issue for: allow for prefix matched metrics
  • (maybe) (rust)tls connections
  • Finish comments

Note: network information events will be in a separate PR by @bgins.

@codecov
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #405 (7167901) into main (af0964b) will increase coverage by 2.37%.
Report is 1 commits behind head on main.
The diff coverage is 82.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #405      +/-   ##
==========================================
+ Coverage   71.88%   74.26%   +2.37%     
==========================================
  Files          71       73       +2     
  Lines        6972     7669     +697     
==========================================
+ Hits         5012     5695     +683     
- Misses       1960     1974      +14     
Files Coverage Δ
homestar-core/src/workflow/input/parse.rs 86.36% <ø> (ø)
homestar-core/src/workflow/instruction.rs 64.86% <100.00%> (ø)
homestar-core/src/workflow/instruction_result.rs 90.24% <100.00%> (ø)
homestar-core/src/workflow/task.rs 91.30% <100.00%> (ø)
homestar-runtime/src/logger.rs 92.00% <100.00%> (+0.10%) ⬆️
homestar-runtime/src/metrics.rs 100.00% <100.00%> (ø)
homestar-runtime/src/metrics/exporter.rs 100.00% <100.00%> (ø)
homestar-runtime/src/metrics/node.rs 96.89% <100.00%> (+27.30%) ⬆️
homestar-runtime/src/network/rpc.rs 77.22% <ø> (ø)
homestar-runtime/src/network/swarm.rs 58.62% <100.00%> (ø)
... and 17 more

... and 2 files with indirect coverage changes

examples/websocket-relay/README.md Outdated Show resolved Hide resolved
homestar-functions/README.md Outdated Show resolved Hide resolved
homestar-runtime/Cargo.toml Show resolved Hide resolved
homestar-runtime/src/network/webserver.rs Outdated Show resolved Hide resolved
@avivash
Copy link
Member

avivash commented Nov 1, 2023

@zeeshanlakhani @bgins is this the branch I should use to test the JSON RPC for workflows?

@zeeshanlakhani zeeshanlakhani force-pushed the zl/jsonrpc branch 4 times, most recently from 68a78ed to 56eedf0 Compare November 2, 2023 07:16
@zeeshanlakhani zeeshanlakhani added the dx Developer experience applications and improvements label Nov 2, 2023
@zeeshanlakhani zeeshanlakhani force-pushed the zl/jsonrpc branch 2 times, most recently from 8d74482 to 6d17195 Compare November 2, 2023 18:37
Includes:
  - re-purposing of feature flags
    * metrics is always a thing (on)
    * monitoring is the gated feature
    * websocket-server flag is gone, we only gate push notifications
  - jsonrpc setup and rpc method register
  - prometheus exposition format to json parser
  - Added todo around split networking config

Notes:
  - will not merge this in until example app has been restored with the client changes
@zeeshanlakhani zeeshanlakhani marked this pull request as ready for review November 6, 2023 21:33
@zeeshanlakhani zeeshanlakhani requested a review from a team as a code owner November 6, 2023 21:33
@zeeshanlakhani
Copy link
Contributor Author

As discussed, @bgins will move around NotifyReceipt to fit within this notification paradigm.

@zeeshanlakhani zeeshanlakhani merged commit 6b88bd9 into main Nov 6, 2023
27 checks passed
@zeeshanlakhani zeeshanlakhani deleted the zl/jsonrpc branch November 6, 2023 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dx Developer experience applications and improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants