Skip to content

feat: provide bincode serde#76

Merged
muse254 merged 6 commits into
mainfrom
feat/provide-bincode-serde
Dec 5, 2025
Merged

feat: provide bincode serde#76
muse254 merged 6 commits into
mainfrom
feat/provide-bincode-serde

Conversation

@muse254
Copy link
Copy Markdown
Contributor

@muse254 muse254 commented Dec 2, 2025

Changes:

  • using bincode serde for ntor::EncryptedType

>(&ctx.get_request_body()) {
Ok(res) => Ok(res),
// deserialize from bincode
match bincode::decode_from_slice(&ctx.get_response_body(), bincode::config::standard()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see you keep avoiding ProxyHandler::parse_request_body (or pingora-router utils), what's your reason?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The init handshake still uses json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The init handshake still uses json

is it related to my above question?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll get the bincode into the utils

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You alluded to us having that conversation/ticket later

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You alluded to us having that conversation/ticket later

Haha, yeah. But I’m honestly curious about your reason for not using the utils? If you feel a utility should be removed because it’s unnecessary, it’s important to look at its usages and the original purpose behind it.

@dtpthao
Copy link
Copy Markdown
Contributor

dtpthao commented Dec 4, 2025

still buggy, have you tested ?

image image

@muse254
Copy link
Copy Markdown
Contributor Author

muse254 commented Dec 4, 2025

I'm having an error when testing the whole flow
Any pointers are welcome(my discord is taking forever to update)
Screenshot 2025-12-03 at 23 17 12

I suspect configs I have; I'm using .env.dev for the proxies and ran make setup_local_dependency which failed on local but make run_layer8server_local works so there might be the disconnect there with ntor certs
Wasted a lot of time and gave up on this

@dtpthao
Copy link
Copy Markdown
Contributor

dtpthao commented Dec 4, 2025

I'm having an error when testing the whole flow Any pointers are welcome(my discord is taking forever to update) Screenshot 2025-12-03 at 23 17 12

I suspect configs I have; I'm using .env.dev for the proxies and ran make setup_local_dependency which failed on local but make run_layer8server_local works so there might be the disconnect there with ntor certs Wasted a lot of time and gave up on this

try to use layer8-docker for auth-server services

@dtpthao
Copy link
Copy Markdown
Contributor

dtpthao commented Dec 4, 2025

I'm having an error when testing the whole flow Any pointers are welcome(my discord is taking forever to update) Screenshot 2025-12-03 at 23 17 12

I suspect configs I have; I'm using .env.dev for the proxies and ran make setup_local_dependency which failed on local but make run_layer8server_local works so there might be the disconnect there with ntor certs Wasted a lot of time and gave up on this

ah, sorry, misread, show me your reverse-proyx env, the ntor server ID should look like this NTOR_SERVER_ID=http://localhost:6193

@muse254
Copy link
Copy Markdown
Contributor Author

muse254 commented Dec 4, 2025

@dtpthao The flow should now work

@muse254 muse254 requested a review from dtpthao December 4, 2025 07:54
Copy link
Copy Markdown
Contributor

@dtpthao dtpthao left a comment

Choose a reason for hiding this comment

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

Update ntor import and interceptor import in spa-frontend, reverse-proxy version and you can merge it

Comment thread reverse-proxy/Cargo.toml Outdated
once_cell = "1.21.3"
dotenv = "0.15.0"
ntor = { git = "https://github.com/globe-and-citizen/ntor.git", tag = "0.1.1" }
ntor = { git = "https://github.com/globe-and-citizen/ntor.git", branch = "feat/provide-bincode-serde"}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just use tag or version

@muse254 muse254 merged commit 646d514 into main Dec 5, 2025
0 of 2 checks passed
@muse254 muse254 deleted the feat/provide-bincode-serde branch December 5, 2025 07:14
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.

2 participants