-
Notifications
You must be signed in to change notification settings - Fork 209
Conversation
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.
lgtm, although I think we need to point to a code sample to make the example relevant, now it doesn't really help anyone to get started with Off-Chain Indexing (and that was my goal with the original example).
be exactly the same for every node with indexing enabled. | ||
|
||
Off-chain database can be read using remote procedure calls (RPC) so it fits the use case of storing | ||
indefinitely growing data without over-consuming the on-chain storage. |
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.
Maybe let's add the code samples to Development Guide
as well - right now, since there is no pointers at all it's really hard to go from "I've learned the concept" to "I want to try it out".
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.
I would prefer to follow Jimmy's suggestion and keep the code in the Recipes.
Edit: this definitely implies we need to make these links easy to find and follow 💯
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.
Thanks input from Tomek and Dan.
Tomek, I commit that I will write a sample code, either in recipe or the Runtime
section below on off-chain index before Jan 15th. If we put code out, we likely need a section of code and explain to readers the context. So I need to experiment with the API myself first, and this takes some work and time. Will let you guys review it once it is done. Then we will add a link to the code section here.
So I prefer keeping this section to be conceptual and show the code in another section. Let me know if you are okay with this, @tomusdrw
@@ -46,3 +46,23 @@ should be implemented to determine what information gets into the chain. | |||
|
|||
For more information on how to use off-chain workers in your next runtime development project, | |||
please refer to our [Development Guide](../runtime/off-chain-workers). | |||
|
|||
# Off-Chain Indexing |
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.
TBH I'm not sure it's fair to bundle Off-Chain Indexing under Off-Chain Workers. These two things can really be though of independently.
I'd suggest renaming the entire article to "Off-Chain Features" or sth, and then having 3 sections that outline 3 independent substrate features:
- Off-Chain Database - a node-local database that every substrate node has, can be accessed over RPC (both writes (unsafe) and reads (safe)).
- Off-Chain Workers - an idea of running off-chain an arbitrary logic defined within the runtime code (when the node is fully synced). Workers can access Off-Chain DB.
- Off-Chain Indexing - a Runtime function to populate Off-Chain DB (opt-in via CLI flag) even during sync.
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.
Thank you both for thinking critically about how we present these concepts to our users 🙏 I would not be opposed to Tomek's suggestion at all, and we could even think about "lifting" this out of the Runtime section and creating a new top-level Off-Chain section or something.
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.
I will follow tomek suggestion of renaming the section as Off-Chain Features
and having 3 children sections inside.
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.
Thank you both 🙏
@@ -46,3 +46,23 @@ should be implemented to determine what information gets into the chain. | |||
|
|||
For more information on how to use off-chain workers in your next runtime development project, | |||
please refer to our [Development Guide](../runtime/off-chain-workers). | |||
|
|||
# Off-Chain Indexing |
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.
Thank you both for thinking critically about how we present these concepts to our users 🙏 I would not be opposed to Tomek's suggestion at all, and we could even think about "lifting" this out of the Runtime section and creating a new top-level Off-Chain section or something.
be exactly the same for every node with indexing enabled. | ||
|
||
Off-chain database can be read using remote procedure calls (RPC) so it fits the use case of storing | ||
indefinitely growing data without over-consuming the on-chain storage. |
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.
I would prefer to follow Jimmy's suggestion and keep the code in the Recipes.
Edit: this definitely implies we need to make these links easy to find and follow 💯
Co-authored-by: Dan Forbes <dan@parity.io>
Co-authored-by: Dan Forbes <dan@parity.io>
@tomusdrw as you suggested, I have rewritten the section and rename it to Off-Chain Features. Let me know what you think and if they are still technically correct 🙏 Thank you all for the review, tomek and dan. |
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.
❤️ looks great, just small clarification about on-chain logic access to off-chain storage would be good.
Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com>
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.
Can you guys please review my changes and make sure I didn't mess anything up? f0db1ab This is a great addition...thank you so much, @tomusdrw and @jimmychu0807 🙏🏻
* refining off-chain indexing * Update docs/knowledgebase/learn-substrate/off-chain-workers.md Co-authored-by: Dan Forbes <dan@parity.io> * Update docs/knowledgebase/learn-substrate/off-chain-workers.md Co-authored-by: Dan Forbes <dan@parity.io> * Expanded the section from off-chain-worker to off-chain-features * Update docs/knowledgebase/learn-substrate/off-chain-features.md Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * update on content to make it more coherent * Fix link * Links and nitpicks * Prettier Co-authored-by: Dan Forbes <dan@parity.io> Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> Co-authored-by: Dan Forbes <dan@danforbes.dev>
* Add validate address options I have added the 2 main tools that Parity has created to validate ss58 addresses. I also added a list of places for developers to look for some more code examples to use to create their own custom validation tools in various languages. @joepetrowski @danforbes * Fix line length * Add validate address options I have added the 2 main tools that Parity has created to validate ss58 addresses. I also added a list of places for developers to look for some more code examples to use to create their own custom validation tools in various languages. @joepetrowski @danforbes * Nitpicks * Updated Terms of Use per Legal (#803) * Updated Terms of Use per Legal * Prettier `npx prettier --write --print-width 100 ./website/pages/en/terms.js` Co-authored-by: Dan Forbes <dan@danforbes.dev> * refining off-chain indexing (#800) * refining off-chain indexing * Update docs/knowledgebase/learn-substrate/off-chain-workers.md Co-authored-by: Dan Forbes <dan@parity.io> * Update docs/knowledgebase/learn-substrate/off-chain-workers.md Co-authored-by: Dan Forbes <dan@parity.io> * Expanded the section from off-chain-worker to off-chain-features * Update docs/knowledgebase/learn-substrate/off-chain-features.md Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> * update on content to make it more coherent * Fix link * Links and nitpicks * Prettier Co-authored-by: Dan Forbes <dan@parity.io> Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> Co-authored-by: Dan Forbes <dan@danforbes.dev> * Bump ini from 1.3.5 to 1.3.7 in /website (#801) Bumps [ini](https://github.com/isaacs/ini) from 1.3.5 to 1.3.7. - [Release notes](https://github.com/isaacs/ini/releases) - [Commits](npm/ini@v1.3.5...v1.3.7) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jimmy Chu <jimmychu0807@gmail.com> * Making most static strings in substrate.dev supporting multi-lingual (#802) * adding <translate> tag for multi-lingual support * Most strings are translatable now * Minor fix of bash syntax in console output * fixing spacing * using relative link so multi-lingual option is kept between navigation * cmd style in console output * Indent all JS to use spacing for indentation * Updated to latest libs * Fix site internal links to respect multi-lingual. * Add validate address options I have added the 2 main tools that Parity has created to validate ss58 addresses. I also added a list of places for developers to look for some more code examples to use to create their own custom validation tools in various languages. @joepetrowski @danforbes * Fix line length * Add validate address options I have added the 2 main tools that Parity has created to validate ss58 addresses. I also added a list of places for developers to look for some more code examples to use to create their own custom validation tools in various languages. @joepetrowski @danforbes * Fix: Comment Edits * Update docs/knowledgebase/advanced/ss58-address-format.md Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> * Update docs/knowledgebase/advanced/ss58-address-format.md Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> * Update docs/knowledgebase/advanced/ss58-address-format.md Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> * Update docs/knowledgebase/advanced/ss58-address-format.md Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> * Update docs/knowledgebase/advanced/ss58-address-format.md Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> * Review and edits for SS58 General pruning to make text more succinct. Slight tweak on structure and linking to be more consistent with other KB articles. Added code comments too. * Revert "Review and edits for SS58 " This reverts commit 532d283. * Apply uncontroversial suggestions from code review Co-authored-by: sacha-l <sacha@parity.io> * fix github suggestions error * Update docs/knowledgebase/advanced/ss58-address-format.md Co-authored-by: sacha-l <sacha@parity.io> * fix: wording and formatting * Update docs/knowledgebase/advanced/ss58-address-format.md Co-authored-by: Dan Forbes <dan@danforbes.dev> Co-authored-by: Imad Arain <57454832+imadarai@users.noreply.github.com> Co-authored-by: Jimmy Chu <jimmy@parity.io> Co-authored-by: Dan Forbes <dan@parity.io> Co-authored-by: Tomasz Drwięga <tomusdrw@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Jimmy Chu <jimmychu0807@gmail.com> Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com> Co-authored-by: Sacha <alexander.lansky@gmail.com> Co-authored-by: sacha-l <sacha@parity.io>
@tomusdrw @danforbes
I have:
Let me know what you think. Thanks