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

feat: track remote pins in progress #1919

Merged
merged 30 commits into from
Oct 4, 2022
Merged

feat: track remote pins in progress #1919

merged 30 commits into from
Oct 4, 2022

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Apr 8, 2022

Closes #1752
Closes #1991

  • Persistance via persistActions
  • Breathing cloud when pending
  • Red cloud when failed
  • Periodic checks per storage with back-off
  • Dismiss failed pin when clicking red cloud (failed)
  • Replace /pins by a list of pending and done pin jobs that can be cleared (similar to the upload drawer)

There are some things you can test via the Storybook. However, there are some aspects, such as dismissing a failed pin on the Files page that I wasn't able to test.

@hacdias
Copy link
Member Author

hacdias commented Apr 27, 2022

A quick update on the pins statuses:

Screenshot 2022-04-27 at 12 04 37

Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @hacdias!

I won't be able to look before I'm AFK for a week, but would be cool if @SgtPooki took it for a spin in the meantime, with a fresh set of eyes. (not a full review, just try to use this and write down and comment on the UX)

@lidel lidel requested a review from SgtPooki April 29, 2022 12:15
@lidel lidel force-pushed the main branch 4 times, most recently from dabaee3 to 7ddf870 Compare June 29, 2022 22:19
@lidel lidel added this to the v2.16 milestone Jul 2, 2022
@lidel
Copy link
Member

lidel commented Jul 2, 2022

@hacdias mind resolving conflicts? I think we should review and ship this, so this work is not lost.

@SgtPooki
Copy link
Member

SgtPooki commented Jul 9, 2022

@juliaxbow another one for you

@hacdias hacdias requested a review from lidel July 10, 2022 06:16
@hacdias hacdias temporarily deployed to Deploy July 10, 2022 06:20 Inactive
@SgtPooki
Copy link
Member

SgtPooki commented Jul 18, 2022

I tried running this locally and got the following error in the following scenarios:

  1. no daemon running
  2. daemon running, then trying to open the "set pinning" modal

image

I have a few changes coming up that resolve these errors.

Note that I added estuary, so I have three pinning services, and no matter what I do, selecting 'estuary' and applying in the "set pinning modal" and then re-opening it, always leaves estuary unchecked:

image

@hacdias hacdias requested a review from a team as a code owner October 3, 2022 08:42
@hacdias hacdias temporarily deployed to Deploy October 3, 2022 08:47 Inactive
@hacdias hacdias temporarily deployed to Deploy October 3, 2022 08:53 Inactive
@hacdias
Copy link
Member Author

hacdias commented Oct 3, 2022

@SgtPooki I created some follow up issues (#2043 #2042) according to your comments. Once CI is green, I will be merging this.

@hacdias hacdias temporarily deployed to Deploy October 3, 2022 09:03 Inactive
@hacdias hacdias requested a review from SgtPooki October 3, 2022 09:12
@hacdias
Copy link
Member Author

hacdias commented Oct 3, 2022

@SgtPooki it seems I need a new approval for this to go through 😄

@SgtPooki SgtPooki temporarily deployed to Deploy October 3, 2022 16:29 Inactive
@SgtPooki SgtPooki temporarily deployed to Deploy October 3, 2022 19:15 Inactive
@hacdias
Copy link
Member Author

hacdias commented Oct 3, 2022

I do not understand. There are no pending changes requested, but I still can't merge.

Uploading Screenshot 2022-10-03 at 21.57.53.png…

@hacdias
Copy link
Member Author

hacdias commented Oct 3, 2022

Ah, @lidel requested changes at some point so we also need @lidel's review.

@SgtPooki SgtPooki temporarily deployed to Deploy October 4, 2022 09:21 Inactive
@hacdias hacdias temporarily deployed to Deploy October 4, 2022 09:25 Inactive
@hacdias hacdias dismissed lidel’s stale review October 4, 2022 16:19

Comments were addressed.

@hacdias hacdias merged commit d3a6524 into main Oct 4, 2022
@hacdias hacdias deleted the feat/track-pins branch October 4, 2022 16:19
@SgtPooki SgtPooki mentioned this pull request Oct 17, 2022
ipfs-gui-bot pushed a commit that referenced this pull request Nov 9, 2022
## [2.20.0](v2.19.0...v2.20.0) (2022-11-09)

 CID `bafybeibjbq3tmmy7wuihhhwvbladjsd3gx3kfjepxzkq6wylik6wc3whzy`

 ---

### Features

* add success notification on "set pinning" action ([#2038](#2038)) ([e410164](e410164))
* track remote pins in progress ([#1919](#1919)) ([d3a6524](d3a6524))
* update to ipfs-geoip v9 ([#2061](#2061)) ([546fb5a](546fb5a))

### Bug Fixes

*  /webtransport breaks status page ([#2057](#2057)) ([ea89e7f](ea89e7f)), closes [#2033](#2033)

### Trivial Changes

* pull new translations ([#2049](#2049)) ([f6062b2](f6062b2))
* pull new translations ([#2056](#2056)) ([e13ff17](e13ff17))
* pull new translations ([#2059](#2059)) ([0bb5bf3](0bb5bf3))
@ipfs-gui-bot
Copy link
Collaborator

🎉 This PR is included in version 2.20.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Unable to set pinning at local: Timeout Files: track pins in progress across node restarts
5 participants