Page MenuHomePhabricator
Differential D13408

[lib] Add action type for processing holders
ClosedPublic
Actions

Authored by bartek on Fri, Sep 20, 6:21 AM.
Tags
None
Referenced Files
F3004124: D13408.id44414.diff
Fri, Oct 18, 12:27 PM
Unknown Object (File)
Mon, Oct 14, 12:44 PM
Unknown Object (File)
Mon, Oct 14, 12:43 PM
Unknown Object (File)
Mon, Oct 14, 12:43 PM
Unknown Object (File)
Sat, Oct 12, 12:15 AM
Unknown Object (File)
Fri, Oct 11, 11:11 PM
Unknown Object (File)
Sun, Oct 6, 9:15 AM
Unknown Object (File)
Thu, Oct 3, 6:04 PM
View All 39 Files
Subscribers
ashoat

Details

Reviewers
kamil
tomek
Commits
rCOMM153dd8771db1: [lib] Add action type for processing holders
Summary

Added action types and updated reducer for the "process holders" action.

  • Holders are marked as PENDING_ESTABLISHMENT or PENDING_REMOVAL, before making Blob calls
  • Afterwards they're either removed or marked as ESTABLISHED.
  • In case of failure, all of them are marked as NOT_ESTABLISHED / NOT_REMOVED. This will be then handled by a handler component (introduced later) for retrying.

Depends on D13403

Test Plan
  • Reducer unit tests
  • Tested together with actual Blob calls

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek created this revision.Fri, Sep 20, 6:21 AM
bartek held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFri, Sep 20, 6:21 AM
bartek added a child revision: D13409: [lib] Add functions to perform Blob Service holder actions.Fri, Sep 20, 6:24 AM
Harbormaster completed remote builds in B31737: Diff 44371.Fri, Sep 20, 6:40 AM
bartek published this revision for review.Fri, Sep 20, 6:43 AM
ashoat added inline comments.Fri, Sep 20, 8:00 AM
lib/actions/holder-actions.js
14–23 ↗(On Diff #44371)

Why aren't these types read-only?

kamil requested changes to this revision.Fri, Sep 20, 8:16 AM
kamil added inline comments.
lib/actions/holder-actions.js
3–4 ↗(On Diff #44371)
7 ↗(On Diff #44371)

nit: I would remove StoreEstablishedHolderPayload and use BlobHashAndHolder directly

14–23 ↗(On Diff #44371)
lib/reducers/holder-reducer.js
63 ↗(On Diff #44371)

I think we should change this, update PROCESS_HOLDERS_FAILED to have failed hashes as payload (similar as here) and only those mark as failed.

Additionally wondering, if we really need pending states after this.

This revision now requires changes to proceed.Fri, Sep 20, 8:16 AM
ashoat added inline comments.Fri, Sep 20, 8:45 AM
lib/reducers/holder-reducer.js
66–71 ↗(On Diff #44371)

Please avoid this: https://www.notion.so/commapp/Use-ternary-conditionals-sparingly-f4ba44a10259403592a1d15440a9847e

bartek updated this revision to Diff 44414.Mon, Sep 23, 4:47 AM

Updated types and failure action

Harbormaster completed remote builds in B31767: Diff 44414.Mon, Sep 23, 5:05 AM
kamil accepted this revision.Mon, Sep 23, 5:09 AM
This revision is now accepted and ready to land.Mon, Sep 23, 5:09 AM
bartek updated this revision to Diff 44419.Mon, Sep 23, 5:14 AM

Newline

bartek mentioned this in D13409: [lib] Add functions to perform Blob Service holder actions.Mon, Sep 23, 5:17 AM
Harbormaster completed remote builds in B31772: Diff 44419.Mon, Sep 23, 5:31 AM
bartek added inline comments.Mon, Sep 23, 7:13 AM
lib/reducers/holder-reducer.js
63 ↗(On Diff #44371)

makes sense, thanks!

Additionally wondering, if we really need pending states after this.

Yes, we need it to avoid unnecessarily start processing new holders if they're already being processed (pending status)

Closed by commit rCOMM153dd8771db1: [lib] Add action type for processing holders (authored by bartek). · Explain WhyMon, Sep 23, 7:24 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM153dd8771db1: [lib] Add action type for processing holders.

Revision Contents
Changeset List

PathSize
lib/
actions/
26 lines
reducers/
82 lines
90 lines
types/
5 lines
26 lines

Diff 44419

View Options

lib/actions/holder-actions.js

Loading...
View Options

lib/reducers/holder-reducer.js

Loading...
View Options

lib/reducers/holder-reducer.test.js

Loading...
View Options

lib/types/holder-types.js

Loading...
View Options

lib/types/redux-types.js

Loading...