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

Improve request/response documentation #5276

Open
nazar-pc opened this issue Mar 28, 2024 · 4 comments
Open

Improve request/response documentation #5276

nazar-pc opened this issue Mar 28, 2024 · 4 comments

Comments

@nazar-pc
Copy link
Contributor

Request/response is a powerful primitive, but could use a better description regarding codec requirements.

I looked at example json and cbor implementations and looks like codec is single-use (I guess a new stream is used for each request/response).

Then I looked into why codec is required to be clonable assuming it can be reused for different streams, but turns out it is cloned every time, limiting reuse of previously allocated buffers (that both json and cbor allocated in read methods every time from scratch.

Would be great if someone with good knowledge of this protocol improved documentation about codec requirements and maybe improved reviewed code looking at efficiency improvements.

For now I'll just follow mentioned codecs as examples.

P.S. Would be great to see https://github.com/paritytech/parity-scale-codec support as well, I'm happy to send a PR if it is welcome.

@jxs
Copy link
Member

jxs commented Mar 29, 2024

Hi Nazar, thanks for opening this.
Do you want to submit a PR improving the doc with the information you learned?
Regarding https://github.com/paritytech/parity-scale-codec support, Codec is pub and therefore can be implemented by a third party lib right?

@nazar-pc
Copy link
Contributor Author

Do you want to submit a PR improving the doc with the information you learned?

Those are my assumptions, I'm not sure they are 100% accurate.

therefore can be implemented by a third party lib right?

Absolutely, but there are two implementations already, so maybe more are welcome?

@jxs
Copy link
Member

jxs commented Apr 1, 2024

I'd prefer leaving third party implementations on separate repos where it's maintainers could update them without me having to review them, but I'd be more than happy to update the README.md to link for them, wdyt Nazar?

@nazar-pc
Copy link
Contributor Author

nazar-pc commented Apr 2, 2024

Makes sense to me

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

No branches or pull requests

2 participants