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

Pinning integration: descope stats from Settings screen #1544

Closed
lidel opened this issue Jul 14, 2020 · 8 comments
Closed

Pinning integration: descope stats from Settings screen #1544

lidel opened this issue Jul 14, 2020 · 8 comments
Labels
effort/hours Estimated to take one or several hours effort/weeks Estimated to take multiple weeks exp/wizard Extensive knowledge (implications, ramifications) required kind/discussion Topical discussion; usually not changes to codebase need/analysis Needs further analysis before proceeding need/maintainers-input Needs input from the current maintainer(s) P0 Critical: Tackled by core team ASAP

Comments

@lidel
Copy link
Member

lidel commented Jul 14, 2020

Initial Settings mockups in WebUI include columns with basic stats: utilized storage as "Files" and "Bandwidth Used":

files-storage-

Problem

Pinning Service API spec does not provide endpoint for reading this information.

Additionally, after sync with @jacobheun @aschmahmann and @alanshaw we decided to make API secrets write-only in go/js-ipfs config for security reasons. This means WebUI won't be able to read API secret and make remote calls on its own.

Solutions

We have two ways of tackling this:

  • weeks: add /stats endpoint to Pinning Service API and then support for it needs to land in go/js-ipfs and webui

or

  • hours: descope this from MVP and remove "Files" and "Bandwidth Used" columns from Settings screen. User will still be able to check stats via three dot menu and a link to pinning service website.

Suggested solution

I don't believe vaue added by those columns is worth additional complexity and additional work on Pinning Service and Core Impl. WG ends – are we ok with descoping them from MVP?

cc @jacobheun @jessicaschilling @rafaelramalho19 for quick temperature check, before I escalate this upstream

@lidel lidel added kind/discussion Topical discussion; usually not changes to codebase P0 Critical: Tackled by core team ASAP need/analysis Needs further analysis before proceeding need/maintainers-input Needs input from the current maintainer(s) exp/wizard Extensive knowledge (implications, ramifications) required effort/hours Estimated to take one or several hours effort/weeks Estimated to take multiple weeks labels Jul 14, 2020
@jacobheun
Copy link

@lidel while it's a very nice feature to have I agree we should not include it in the initial MVP work. It's not necessary for initial functionality and would be easy to add later without needing to change the existing spec.

@jessicaschilling
Copy link
Contributor

+1, but let's be sure to consider thoroughly for post-MVP work.

@momack2
Copy link
Contributor

momack2 commented Jul 14, 2020

I’d really like to keep this in scope if possible. It helps answer the question of “which pinning service am I using) without having to go check a bunch of different sites, which would be really annoying. If we need to replace - is there something we can replace it with to give that signal?

@jessicaschilling
Copy link
Contributor

@momack2 Not suggesting to remove the rows themselves, rather just the "files" and "bandwidth used" columns.

@lidel
Copy link
Member Author

lidel commented Jul 15, 2020

Context: we discussed this briefly during yesterday's review call.
My takeaway was we are ok with removing those specific storage/bandwidth stats, as long we replace them with something that provides "usage signal" to the user, like @momack2 said.

Potential candidate is "pin count" – number coud be read from count field returned by GET /pins?status=pinned in Pinning Service API (but we need to figure out how to expose this value over go-ipfs' HTTP API).

Separately, we could discuss adding PinStatus.size field to indicate the size pinned DAG uses at the pinning service, but it brings similar question on how to expose it to WebUI from go-ipfs.

@jacobheun where should we track mapping from Pinning Service API to go-ipfs commands and HTTP API? Even a hackpad would help with resolving gaps like ones described above.

@jessicaschilling
Copy link
Contributor

+1 for "pin count" being a good usage signal, at least for this version of the API. I suspect some others will emerge in time as more pinning services come on as adopters.

@jessicaschilling
Copy link
Contributor

@lidel Are we OK to close this issue now that we're proceeding with "pin count"?

@lidel
Copy link
Member Author

lidel commented Sep 15, 2020

Yes, I talked with @aschmahmann and iiuc we will have some type of stat command that returns the number of pins in each state (queued/pinning/pinned/failed)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/hours Estimated to take one or several hours effort/weeks Estimated to take multiple weeks exp/wizard Extensive knowledge (implications, ramifications) required kind/discussion Topical discussion; usually not changes to codebase need/analysis Needs further analysis before proceeding need/maintainers-input Needs input from the current maintainer(s) P0 Critical: Tackled by core team ASAP
Projects
None yet
Development

No branches or pull requests

4 participants