-
Notifications
You must be signed in to change notification settings - Fork 14
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
Port features from pl-diagnose #24
Comments
@laurentsenta @aschmahmann I think we should port ipfs-check to pl-diagnose tbh. The front-end code in pl-diagnose is much more friendly for devs. I haven't looked at the backend of pl-diagnose, but ipfs-check backend is pretty straightforward. However, here are some problems I see with this repo:
|
🤣 I'm sure. I haven't done any JS UX much of anything, the only reason the website doesn't look awful at the moment is someone with better skills than me (olizilla) spruced it up. @laurentsenta's front end for working through the steps is also just better (e.g. resolves #6). IIRC the pl-diagnose backend is very slow compared to the one here (probably due to not using the accelerated DHT client). So I wouldn't want to swap backends until that's resolved. Side note about the backend: There might be users other than people who visit check.ipfs.network that hit the backend. If we break the backend that's probably fine, but maybe cut a release of the repo with some release notes documenting the change or at least describe the changes well in a commit message. Also ping @TheDiscordian since IIRC he'd have a bot to update. |
😄 From your comments @aschmahmann, @SgtPooki it looks like we could start with porting the API here and eventually merge the pl-diagnose frontend. There's also an env variable in pl-diagnose that we can use to hit ipfs-check when the backend is ready. @aschmahmann, we'll add more endpoints to that project, do you have a preference/recommendation for the approach (maybe a framework, library, or sticking to stdlib)? |
At this point with #6, CID retrievability diagnostic functionality from pl-diagnose have been ported. Since the goal of this tool is to test retrievability. I haven't ported the lower level/more granular checks to keep things simple for a user: they can either run a check with just a CID or with a CID and a multiaddr (either PeerID or full multiaddr with an IP etc.)
I would be in favour of closing this issue. Unless you think it's critical. Please share your feedback |
Is anyone paying to keep pl-diagnose running, or is it free? Would we miss the functionality (only identify and UX flow now..) if it was gone? How many folks are using it monthly vs ipfs-check? It would be nice to have a single diagnostic tool, but I'd be fine to close this until we have more motivation and time to work on a consolidated tool. |
We implemented pl-diagnose during launchpad as a "ipfs-check but easier to use",
We want to merge its features back into ipfs-check.
https://github.com/laurentsenta/pl-diagnose
API endpoints:
https://github.com/laurentsenta/pl-diagnose/blob/32be4cf13db63da02a5f626215f849c60f80a1b2/backend/main.go#L31-L49
/find?cid={cid}
: Find a piece of content in the DHT/find-peer?addr={peer-multiaddress}
: Find a peer in the DHT/identify?addr={peer-multiaddress}
: Dial into the peer and use the identify protocol/bitswap?addr={peer-multiaddress}&cid={cid}
: Dial into the peer and try getting the content identified by the cidOutputs & Implementations can be found in the daemon file.
UI / UX
Not sure what we want to do with the frontend. It made sense to use something familiar during launchpad,
a few things probably worth saving:
❓ Need help with this result? Ask for help on github
that generates an issue for the user.The text was updated successfully, but these errors were encountered: