-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add websockets stream api docs #1074
base: main
Are you sure you want to change the base?
Add websockets stream api docs #1074
Conversation
* Added a single page overiview docs * Added a multi-page detailed docs for each message type
@illia-malachyn is attempting to deploy a commit to the Flow Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
docs/networks/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api.md
Outdated
Show resolved
Hide resolved
@illia-malachyn the deployed vercel from a few days ago is here. With the left nav we probably want a top label that is more readable than "websockets-stream-api". This nav option is also distinct from the written title of 'Websockets Stream API'. My guess is the intention was for the sub pages to be under that root, not the 'websockets-stream-api' one ![]() ![]() |
Nav looks good in latest vercel deployment. Looks like there's still more to be added and we should wait to merge this only once the functionality is deployed and live. |
docs/networks/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api.md
Outdated
Show resolved
Hide resolved
...s/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/overview.md
Outdated
Show resolved
Hide resolved
...s/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/overview.md
Outdated
Show resolved
Hide resolved
...s/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/overview.md
Outdated
Show resolved
Hide resolved
...s/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-message.md
Outdated
Show resolved
Hide resolved
...s/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-message.md
Outdated
Show resolved
Hide resolved
...access-onchain-data/access-nodes/accessing-data/websockets-stream-api/unsubscribe-message.md
Outdated
Show resolved
Hide resolved
...onchain-data/access-nodes/accessing-data/websockets-stream-api/list-subscriptions-message.md
Outdated
Show resolved
Hide resolved
...s/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-message.md
Outdated
Show resolved
Hide resolved
...s/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-message.md
Outdated
Show resolved
Hide resolved
@illia-malachyn you will need to merge from main since the project structure for the docs in this area changed. Also, we need to make sure this section is also updated: https://developers.flow.com/networks/access-onchain-data#subscriptions |
Created separate issue #1150 to fix the last part you mentioned, as I see we do not have any description of new gRPC subscriptions and do not mark the old implementation as deprecated. |
...ps/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added a few more comments. otherwise this is good
...onchain-data/access-nodes/accessing-data/websockets-stream-api/list-subscriptions-message.md
Outdated
Show resolved
Hide resolved
...s/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/overview.md
Outdated
Show resolved
Hide resolved
...s/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/overview.md
Outdated
Show resolved
Hide resolved
...s/node-ops/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/overview.md
Outdated
Show resolved
Hide resolved
...s/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-message.md
Outdated
Show resolved
Hide resolved
...ps/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
...ps/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
...ps/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
...ps/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
...ps/access-onchain-data/access-nodes/accessing-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Peter Argue <[email protected]>
@peterargue fixed all remarks. |
docs/networks/access-onchain-data/websockets-stream-api/subscribe-message.md
Outdated
Show resolved
Hide resolved
docs/networks/access-onchain-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
docs/networks/access-onchain-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
docs/networks/access-onchain-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
docs/networks/access-onchain-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
docs/networks/access-onchain-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
We need to add a page with common error codes and maybe examples so that clients can handle them properly |
As we are polishing websockets and some errors may come up, I'd recommend you write an automated subscription script for every topic. By doing this, we could run all of them after each bugfix and see we didn't break anything in case integration tests don't cover some logic. This would work as an additional layer of testing as well as future documentation for clients. Here's a small example you can use https://github.com/The-K-R-O-K/flow-stream-api-example/blob/main/template.js I image sth like that:
Despite the fact that this is out of the scope of this issue and we have a separate issue for it. I can see no reason why to not do it right now. It is faster than testing websockets manually. |
docs/networks/access-onchain-data/websockets-stream-api/subscribe-topics.md
Outdated
Show resolved
Hide resolved
Yeah, it makes sense. Would agree with you, that that will add an extra layer of testing from the client perspective |
Looking good guys thanks! Maybe just a couple of nits left to address above?
Yeah, for postman just one simple example would do. Then I can add an AWS example next to it @illia-malachyn |
@franklywatson Done it. Added "Postman UI example" and "Common errors" pages. |
Thanks guys, looks good @illia-malachyn @Guitarheroua . I think we're done here! Once we've released the functionality will merge this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple small comments, but otherwise this looks good.
one I think is a change we should make in the error handling implementation.
docs/networks/access-onchain-data/websockets-stream-api/common-errors.md
Outdated
Show resolved
Hide resolved
docs/networks/access-onchain-data/websockets-stream-api/common-errors.md
Outdated
Show resolved
Hide resolved
Co-authored-by: Peter Argue <[email protected]>
"topic": "account_statuses", | ||
"payload": { | ||
"block_id": "ab20d1a3574177e69636eea73e7db4e74cffb2754cb14ca0bf18c2b96e8b68b9", | ||
"height": "106219247", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this have a timestamp?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint does not have a timestamp in the gRPC version, and as we use the same functionality inside Websockets, this does not have it either. Should it be present? If yes, I could create an issue for this.
Closes onflow/flow-go#6644
This is in progress and is postponed till we finish with websockets epic. However, you can review it and write notes about it.
What to add to: