Feat(cli): Clean up pinning CLI flags #1012
Conversation
| @@ -269,8 +269,8 @@ class Ceramic implements CeramicApi { | |||
|
|
|||
| const pinStoreProperties = { | |||
| networkName: networkOptions.name, | |||
| pinsetDirectory: config.pinsetDirectory, | |||
| pinningEndpoints: config.pinningEndpoints, | |||
| pinsetDirectory: config.stateStoreDirectory, | |||
There was a problem hiding this comment.
I wasn't sure if I should also rename this args for the PinStoreFactory. The PinStoreFactory tries really hard to be flexible and agnostic about what the actually underlying pinning backend service is, but there would need to be more work done to make it actually possible to provide a different PinningBackend. Also as the entire PinningBackend API is built around CIDs, (and doesn't even give a way to load the IPFS record contents) it's hard to imagine any other backend other than IPFS.
I kind of feel like we have the wrong level of abstraction here. We have all this low-level machinery to allow different PinningBackends, except that capability is never exposed to higher layers and we'll probably never have another implementation. Meanwhile we have hard-coded logic to use a LevelStateStore, and here we are wanting a new StateStore implementation.
Anyway, I think for this PR it makes sense just to focus on the CLI flags, and then my next PR will have to make some bigger changes to the PinStoreFactory to allow plugging in different StateStores. I am however getting worried about this work colliding with the work that @ukstv is also doing in this area and causing some gnarly merge conflicts
No description provided.