Skip to content

ipfs@0.41 update: option 2 #385

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

Merged
merged 9 commits into from
Feb 21, 2020

Conversation

zebateira
Copy link
Contributor

Second option for dealing with the new js ipfs api that always returns iterables.
This second approach uses the it-all package from @achingbrain.

Screen Shot 2020-02-17 at 7 06 01 PM

  • A bit more clean but it might be confusing for users to understand where to use the all function

See original WIP PR: #381

@zebateira zebateira mentioned this pull request Feb 17, 2020
@zebateira zebateira force-pushed the fix/ipfs-0.41-upgrade-option-2 branch from de2fd6c to 2a58f54 Compare February 18, 2020 11:09
@zebateira zebateira force-pushed the fix/ipfs-0.41-upgrade-option-2 branch from 7e976fb to 163e859 Compare February 18, 2020 13:17
@terichadbourne
Copy link
Member

@zebateira I've only had time to look at the first real lesson here (http://localhost:3000/#/regular-files-api/03), but a few thoughts:

  • This is going to be too long with the "working w/ files in IPFS" section built in - would it fit in the previous lesson or make sense as its own?

  • Unanswered question: Does it still return an async iterable even if I only add one file? (I think probably yes based on playing with the challenge, but we should be explicit.)

  • We should state that the thing we're saying about async iterables is only valid after version X and be clear about what happened in previous versions.

  • I think the concept of presenting both options in the same lesson is fine, but we could do more with subheads and additional explanation to make it a bit more clear. So the general vibe would be something like this, but more accurate and with real words:

Starting with version X of IPFS, the add method returns an async iterable (link out to external documentation on this javascript concept) by default. This is super cool because if you're working with huge data sets, you can start processing them as they flow in rather than waiting for all the results to come back (link to Alan's blog post on why this is cool). In the old version, the results of add came back in X format and you had to use a special nameOfAddStreamMethod method to access this cool feature, but now you get that right in the add method.

There are two little complications to the data being returned as an sync iterable: If you're using small datasets, like we'll do here in ProtoSchool, you might just want to know when the job is done. Also, it's kinda hard to work with async iterables in javascript so you'll need to do a little bit of extra work to turn the data into an array. There are two ways to transform the async iterables produced by IPFS into an array:

Option 1: Use a loop in JavaScript
You can loop through an async iterable and push its results into an array, like this:
first example code you showed

Option 2: Use the it-all package
If you want the results gathered up into an array but don't want to do the work yourself, you're in luck. You can pass an async iterable produced by IPFS into the all method from the it-all package (link to npm site) to have it magically transformed into an array. (Don't forget to import the package first!)
second example code you showed
We think this it-all package is pretty handy, so it's the option we'll recommend throughout the lessons in this tutorial.

No matter which of these methods you choose, you'll end up with an array of objects of X format... And this array of objects is exactly the same as what would have been returned by IPFS before version X.

  • It looks like you haven't updated the exercise.md file at all to include hints about needing to change the async iterable to an array.

  • Why does the it-all package get imported as all instead of it-all - is that what the npm docs would show and what the IPFS peeps recommend? Does the it in it-all stand for iterable?

  • I can't tell from the comment here that I'm supposed to put my code INSIDE the parens of the all method. I'm also not convinced we shouldn't make them do the work of setting up all themselves the first time around so they'll remember it. What would you think of just using reminders in the exercise md and the sample code to get them to do this, and making the challenge be specifically to use the all method to do it?
    image

  • We should pretend that for every lesson in this tutorial, someone will try to answer the challenge using code that would have worked in a previous version of IPFS. If this is an example of that, then we should have more detailed logging if possible to say "that would have worked in the old version" or "that returned an async iterable, but we need an array so we can work the data - try one of the methods above to turn the async iterable into an array"

image

  • Do think there would be any benefit to keeping the existing tutorial under a new name/URL for people to reference if they need to learn about the older format? Or shall we just refer them to some sort of docs about the old format?

  • If we didn't have the logging thing built to return other text, and it could just show me the results of adding a bunch of files to IPFS, what would I see? Is it something illegible? Just the words async Iterable? Wondering if we should show a sample output of that somewhere here before introducing the ways to turn it into an array.

Sorry I didn't have time to get to more of this, but happy to chat tomorrow about these first impressions and then put in some more time on it.

@zebateira
Copy link
Contributor Author

This is going to be too long with the "working w/ files in IPFS" section built in - would it fit in the previous lesson or make sense as its own?

Yeah, I suggest separating into two lessons. I'm not sure what kind of separation should we do though.

Unanswered question: Does it still return an async iterable even if I only add one file? (I think probably yes based on playing with the challenge, but we should be explicit.)

Yeah, it always returns an async iterable.

We should state that the thing we're saying about async iterables is only valid after version X and be clear about what happened in previous versions.

I don't think this is necessary. js-ipfs is still in alpha and changes are bound to happen, and teaching old versions is not necessary. I agree that we can mention that from version 0.41 onwards we use Async Iterables, but I would just add a very small sentence with a link to the old docs or the migration guide, and no need to explain in ProtoSchool what is the change or what happened in the previous version, but let me know if there is a reason why we need to mention this.

@zebateira
Copy link
Contributor Author

Why does the it-all package get imported as all instead of it-all - is that what the npm docs would show and what the IPFS peeps recommend? Does the it in it-all stand for iterable?

It's just the name of the variable. From the docs the all name is used and I thought it was appropriate.

I can't tell from the comment here that I'm supposed to put my code INSIDE the parens of the all method. I'm also not convinced we shouldn't make them do the work of setting up all themselves the first time around so they'll remember it. What would you think of just using reminders in the exercise md and the sample code to get them to do this, and making the challenge be specifically to use the all method to do it?

Agreed to make this lesson the first one where they need to use the it-all package. Would this mean in the future we will include the all() call? Or would we leave the user to use it if they want in future lessons?

@zebateira
Copy link
Contributor Author

We should pretend that for every lesson in this tutorial, someone will try to answer the challenge using code that would have worked in a previous version of IPFS. If this is an example of that, then we should have more detailed logging if possible to say "that would have worked in the old version" or "that returned an async iterable, but we need an array so we can work the data - try one of the methods above to turn the async iterable into an array"

I'm working on trying to understand if we can test that the result is an iterable - if it's possible then we can do this.

@zebateira
Copy link
Contributor Author

Do think there would be any benefit to keeping the existing tutorial under a new name/URL for people to reference if they need to learn about the older format? Or shall we just refer them to some sort of docs about the old format?

Yeah, I think just a reference to the old docs would be enough. We shouldn't be teaching an older version of js-ipfs.

@zebateira
Copy link
Contributor Author

If we didn't have the logging thing built to return other text, and it could just show me the results of adding a bunch of files to IPFS, what would I see? Is it something illegible? Just the words async Iterable? Wondering if we should show a sample output of that somewhere here before introducing the ways to turn it into an array.

I'm trying to figure that out now.

@zebateira zebateira merged commit 544abff into fix/ipfs-0.41-upgrade Feb 21, 2020
@zebateira zebateira deleted the fix/ipfs-0.41-upgrade-option-2 branch February 21, 2020 14:31
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