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

H-3709, H-3746, H-3747: HaRPC TypeScript implementation #5836

Merged
merged 28 commits into from
Dec 9, 2024

Conversation

indietyp
Copy link
Member

@indietyp indietyp commented Dec 7, 2024

🌟 What is the purpose of this PR?

This implements the - to be autogenerated - client for the echo service, squashes some bugs that happened and integrates everything with the hash-api.

The RPC client is gated - like the server implementation - under an environment variable: HASH_RPC_ENABLED = true.

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • affected the execution graph, and the turbo.json's have been updated to reflect this

📹 Demo

@github-actions github-actions bot added area/deps Relates to third-party dependencies (area) area/apps > hash* Affects HASH (a `hash-*` app) area/apps > hash-api Affects the HASH API (app) area/libs Relates to first-party libraries/crates/packages (area) type/eng > backend Owned by the @backend team area/apps labels Dec 7, 2024
turborepo doesn’t support it :/
@indietyp indietyp requested a review from CiaranMn December 7, 2024 22:27
@vilkinsons vilkinsons changed the title H-3709, H-3746, H-3747: Working TypeScript implementation H-3709, H-3746, H-3747: HaRPC TypeScript implementation Dec 7, 2024
Copy link

codecov bot commented Dec 7, 2024

Codecov Report

Attention: Patch coverage is 18.85965% with 185 lines in your changes missing coverage. Please review.

Project coverage is 22.97%. Comparing base (417362f) to head (29a2539).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...harpc/client/typescript/src/binary/MutableBytes.ts 22.53% 55 Missing ⚠️
...@local/harpc/client/typescript/src/net/Response.ts 12.12% 29 Missing ⚠️
libs/@local/graph/sdk/typescript/src/harpc.ts 0.00% 24 Missing ⚠️
...pc/client/typescript/src/net/internal/transport.ts 7.69% 24 Missing ⚠️
...rc/wire-protocol/stream/ResponseFromBytesStream.ts 11.11% 24 Missing ⚠️
.../@local/harpc/client/typescript/src/ClientError.ts 0.00% 11 Missing ⚠️
...ocal/harpc/client/typescript/src/net/Connection.ts 11.11% 8 Missing ⚠️
...s/@local/harpc/client/typescript/src/net/Client.ts 40.00% 6 Missing ⚠️
...ent/typescript/src/wire-protocol/models/Payload.ts 50.00% 1 Missing and 1 partial ⚠️
...lient/typescript/src/net/internal/networkLogger.ts 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5836      +/-   ##
==========================================
- Coverage   23.00%   22.97%   -0.03%     
==========================================
  Files         568      571       +3     
  Lines       19160    19300     +140     
  Branches     2715     2732      +17     
==========================================
+ Hits         4408     4435      +27     
- Misses      14700    14812     +112     
- Partials       52       53       +1     
Flag Coverage Δ
apps.hash-ai-worker-ts 1.32% <ø> (ø)
apps.hash-api 1.16% <ø> (ø)
local.harpc-client 62.15% <21.07%> (-3.04%) ⬇️
local.hash-backend-utils 8.80% <ø> (ø)
local.hash-graph-sdk 58.62% <0.00%> (-41.38%) ⬇️
local.hash-isomorphic-utils 0.99% <ø> (ø)
local.hash-subgraph 24.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@indietyp
Copy link
Member Author

indietyp commented Dec 8, 2024

I am unsure why CI fails for hash-graph-sdk it works on my machine with 0 issues and the error seems... odd.

Same with the import issue, I might be doing something wrong with the build step, but I am unsure as to what that would be.

Copy link
Member

@CiaranMn CiaranMn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Bilal, exciting to have our first HaRPC call working!

A few small comments

indietyp and others added 2 commits December 9, 2024 11:59
Co-authored-by: Ciaran Morinan <37743469+CiaranMn@users.noreply.github.com>
…ithub.com:hashintel/hash into bm/h-3709-typescript-manual-client-implementation
@indietyp
Copy link
Member Author

indietyp commented Dec 9, 2024

CI for hash-graph-sdk and I don't know why. @CiaranMn any idea? The lint is passing for harpc-client, but not hash-graph-sdk, for code that resides in harpc-client and linting succeeds locally.

@CiaranMn
Copy link
Member

CiaranMn commented Dec 9, 2024

CI for hash-graph-sdk and I don't know why. @CiaranMn any idea? The lint is passing for harpc-client, but not hash-graph-sdk, for code that resides in harpc-client and linting succeeds locally.

I merged in main, if it fails locally now then it's some PR that has merged since you last updated which has caused an issue

@indietyp indietyp enabled auto-merge December 9, 2024 11:58
@indietyp indietyp added this pull request to the merge queue Dec 9, 2024
Copy link
Contributor

github-actions bot commented Dec 9, 2024

Benchmark results

@rust/hash-graph-benches – Integrations

representative_read_entity

Function Value Mean Flame graphs
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1 $$16.4 \mathrm{ms} \pm 136 \mathrm{μs}\left({\color{lightgreen}-29.242 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2 $$17.1 \mathrm{ms} \pm 209 \mathrm{μs}\left({\color{gray}2.55 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1 $$16.3 \mathrm{ms} \pm 177 \mathrm{μs}\left({\color{gray}3.18 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1 $$16.4 \mathrm{ms} \pm 199 \mathrm{μs}\left({\color{gray}0.542 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1 $$16.5 \mathrm{ms} \pm 223 \mathrm{μs}\left({\color{gray}0.949 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1 $$17.4 \mathrm{ms} \pm 166 \mathrm{μs}\left({\color{gray}3.27 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1 $$15.7 \mathrm{ms} \pm 160 \mathrm{μs}\left({\color{gray}-4.419 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1 $$15.7 \mathrm{ms} \pm 166 \mathrm{μs}\left({\color{lightgreen}-31.874 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1 $$15.8 \mathrm{ms} \pm 170 \mathrm{μs}\left({\color{gray}-1.776 \mathrm{\%}}\right) $$ Flame Graph

representative_read_multiple_entities

Function Value Mean Flame graphs
entity_by_property depths: DT=255, PT=255, ET=255, E=255 $$65.4 \mathrm{ms} \pm 271 \mathrm{μs}\left({\color{gray}0.196 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=0, E=0 $$37.7 \mathrm{ms} \pm 158 \mathrm{μs}\left({\color{gray}-1.056 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=2, PT=2, ET=2, E=2 $$55.7 \mathrm{ms} \pm 216 \mathrm{μs}\left({\color{gray}-0.246 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=0, E=2 $$42.0 \mathrm{ms} \pm 176 \mathrm{μs}\left({\color{gray}-0.284 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=0, ET=2, E=2 $$47.6 \mathrm{ms} \pm 195 \mathrm{μs}\left({\color{gray}0.264 \mathrm{\%}}\right) $$ Flame Graph
entity_by_property depths: DT=0, PT=2, ET=2, E=2 $$52.0 \mathrm{ms} \pm 218 \mathrm{μs}\left({\color{gray}-2.026 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=255, PT=255, ET=255, E=255 $$107 \mathrm{ms} \pm 530 \mathrm{μs}\left({\color{gray}-0.607 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=0, E=0 $$42.0 \mathrm{ms} \pm 254 \mathrm{μs}\left({\color{gray}-2.291 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=2, PT=2, ET=2, E=2 $$97.6 \mathrm{ms} \pm 481 \mathrm{μs}\left({\color{gray}0.059 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=0, E=2 $$80.4 \mathrm{ms} \pm 215 \mathrm{μs}\left({\color{gray}-0.211 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=0, ET=2, E=2 $$89.2 \mathrm{ms} \pm 236 \mathrm{μs}\left({\color{gray}-1.894 \mathrm{\%}}\right) $$ Flame Graph
link_by_source_by_property depths: DT=0, PT=2, ET=2, E=2 $$93.8 \mathrm{ms} \pm 413 \mathrm{μs}\left({\color{gray}-0.726 \mathrm{\%}}\right) $$ Flame Graph

representative_read_entity_type

Function Value Mean Flame graphs
get_entity_type_by_id Account ID: d4e16033-c281-4cde-aa35-9085bf2e7579 $$1.37 \mathrm{ms} \pm 3.43 \mathrm{μs}\left({\color{gray}-0.123 \mathrm{\%}}\right) $$ Flame Graph

scaling_read_entity_complete_one_depth

Function Value Mean Flame graphs
entity_by_id 50 entities $$5.49 \mathrm{s} \pm 827 \mathrm{ms}\left({\color{red}1939 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 5 entities $$26.2 \mathrm{ms} \pm 115 \mathrm{μs}\left({\color{gray}-0.197 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1 entities $$20.0 \mathrm{ms} \pm 110 \mathrm{μs}\left({\color{gray}-4.097 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10 entities $$36.5 \mathrm{ms} \pm 109 \mathrm{μs}\left({\color{lightgreen}-25.653 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 25 entities $$178 \mathrm{ms} \pm 1.00 \mathrm{ms}\left({\color{gray}-2.116 \mathrm{\%}}\right) $$ Flame Graph

scaling_read_entity_linkless

Function Value Mean Flame graphs
entity_by_id 1 entities $$1.91 \mathrm{ms} \pm 5.04 \mathrm{μs}\left({\color{gray}-0.559 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 100 entities $$2.07 \mathrm{ms} \pm 7.02 \mathrm{μs}\left({\color{gray}0.812 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10 entities $$1.92 \mathrm{ms} \pm 4.18 \mathrm{μs}\left({\color{gray}-0.373 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1000 entities $$3.07 \mathrm{ms} \pm 10.5 \mathrm{μs}\left({\color{red}7.66 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10000 entities $$13.4 \mathrm{ms} \pm 53.6 \mathrm{μs}\left({\color{gray}-1.060 \mathrm{\%}}\right) $$ Flame Graph

scaling_read_entity_complete_zero_depth

Function Value Mean Flame graphs
entity_by_id 50 entities $$4.98 \mathrm{ms} \pm 17.4 \mathrm{μs}\left({\color{red}20.0 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 5 entities $$1.93 \mathrm{ms} \pm 5.25 \mathrm{μs}\left({\color{gray}-0.798 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 1 entities $$1.91 \mathrm{ms} \pm 4.98 \mathrm{μs}\left({\color{gray}-0.671 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 10 entities $$2.12 \mathrm{ms} \pm 14.0 \mathrm{μs}\left({\color{gray}-1.284 \mathrm{\%}}\right) $$ Flame Graph
entity_by_id 25 entities $$2.71 \mathrm{ms} \pm 11.3 \mathrm{μs}\left({\color{lightgreen}-18.170 \mathrm{\%}}\right) $$ Flame Graph

Merged via the queue into main with commit a0eea11 Dec 9, 2024
166 checks passed
@indietyp indietyp deleted the bm/h-3709-typescript-manual-client-implementation branch December 9, 2024 12:22
Copy link

sentry-io bot commented Dec 11, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ Error: invalid ip address GET /rpc/echo View Issue

Did you find this useful? React with a 👍 or 👎

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/apps > hash* Affects HASH (a `hash-*` app) area/apps > hash-api Affects the HASH API (app) area/apps area/deps Relates to third-party dependencies (area) area/libs Relates to first-party libraries/crates/packages (area) type/eng > backend Owned by the @backend team
Development

Successfully merging this pull request may close these issues.

2 participants