theobat
commented
7 years ago going to give this a shot
Closed haadcode closed 7 years ago
theobat
commented
7 years ago going to give this a shot
haadcode
commented
7 years ago Fantastic! Thanks k you! Let me know if you run into any problems and I'd be glad to help.
This require to change the startup sequence so that we start the daemon AFTER the login screen, but we can worry about that after you've gotten started.
theobat
commented
7 years ago Just a few architecture questions to follow up :
haadcode
commented
7 years ago the default config object for the ipfs daemon has to move out of its current location
Probably should put it in the UI as a const (or json file which webpack can pick up).
the ipfs daemon sequence has to move after the LoginView, should I dedicate an independent reflux action to this or is it sufficient to make it happen in the connect action
Use what makes sense. There's quite a few changes to the startup sequence with this, so I'm not certain how it should work.
How I see the sequence would work is: 1) open electron window and load the app (main.js), currently opens "loading.html" first, 2) app checks a global/window variable "areWeInElectron" and if we are, send a message via the IPC to Electron and ask for a running instance of IPFS, if electron doesn't have one, display login screen OR if electron has one, emit 'daemon' event via IPC (ipcMain in index.js), 3) in the login screen , the user can change the configuration variables and once the user has entered their name, send a 'start' message to electron's main process via IPC which will start a daemon with the configuration options provided from the UI, 4) once daemon has started, electron's main process will send a 'daemon' event to the app and upon receiving that event, the app now has an IPFS daemon and can call 'connect' action and the rest of the flow should work as it is now.
Now, all this needs to work also with js-ipfs with which the daemon is not started in the electron's main process but in the UI process, so whenever the IPC is called for electron, we'd need to have something else for creating the daemon fully in the browser. Again, not sure how exactly this would work in the code.
Let me know what you think @theobat!
There's a lot there and this is not an easy change as it touched pretty much everything across the layers. If you feel there's something unclear or you're having hard time to understand what the code does and where, please let me know. Also, if you have something already forked, make sure to push your WIP code and I can take a look and help when needed.
haadcode
commented
7 years ago Another thought: we should maybe consider putting the daemon related things to its own Store. This way there's one place where we manage the state of the daemon (rest of the app only need to know if we have a daemon or not), and we can for example re-start the daemon in case it crashes.
theobat
commented
7 years ago I am not sure I understand this part of your post :
2) app checks a global/window variable "areWeInElectron" and if we are, send a message via the IPC to Electron and ask for a running instance of IPFS, if electron doesn't have one, display login screen OR if electron has one, emit 'daemon' event via IPC (ipcMain in index.js)
I thought the running instance of ipfs was the ipfs daemon itself you seem to imply we can get an ipfs instance without its daemon ? Also, Is this specifically related to the case when we already have an ipfs daemon running in the background manually fired or for some other reason) ? (this is just for a better understanding) If that's the case, is it currently taking a manual ipfs daemon into account or is that a new feature ?
My understanding was overall the same as you (except for that one thing):
Another thought: we should maybe consider putting the daemon related things to its own Store. This way there's one place where we manage the state of the daemon (rest of the app only need to know if we have a daemon or not), and we can for example re-start the daemon in case it crashes.
:+1:
theobat
commented
7 years ago Actually I'm having a hard time at moving away the ipfsDaemons settings as they are relying on two path that are computed using electron app dependency. I don't see how it's possible to get these path in the browser scenario. And I think it's actually preventing ipfs-daemon to interface with js-ipfs but I could be wrong, I'm gonna check all that. If you have any pointer so that I can get a better understanding (faster) of how js-ipfs actually store blocks (and where ?) in a browser situation that would be a tremendous help.
By the way the AppDataDir path does not seem to be used at all in ipfs-daemon
Also, as I get deeper into the code and the ui goal of this, I think the real work is on moving the ipfs daemon thing (while not making any other change) It might end up in two different PRs but we'll see
haadcode
commented
7 years ago @theobat you're right AppDataDir is not used anymore and can be cut. js-ipfs daemon uses IndexedDB in the browser to store the blocks and path to that dir can be given as an argument to the constructor. We're in process to unify both js-ipfs-api and js-ipfs and make it really easy to start the daemon: https://github.com/ipfs/js-ipfs/issues/556#issuecomment-257121112.
What we could do is that we either store the defaults to a file, read them on the Electron side at startup and pass them to the browser when the login screen opens. Or just do that for the path. Or we could leave the data path empty in the config screen, send a null for the path when the user logs in, and if it's null, get the path like you get it now. I'd prefer the first option I think. Does that makes sense?
theobat
commented
7 years ago Ok I get it now, the default configs have to go from the main process (because app variable is needed initially) to the ui (because people have to know what they change) and then this (possibly modified with the ui) object goes back to the main process (from renderer) to start the daemon with electron (in case it's a native deamon that has to be started otherwise the IpfsDaemonStore fire the js-ipfs one). Thanks very much for your time and explaination @haadcode My only remaining question is: Will there be some pointless configs for js-ipfs that are required for go-ipfs and vice-versa ? (I'm goin to get there to see by myself but right now it's a bit too much, and I'll only do it for the native/electron fired app, but I'd like to keep the answer to this question in mind so that we won't have too much trouble in setting up js-ipfs daemon)
haadcode
commented
7 years ago Great to hear!
Will there be some pointless configs for js-ipfs that are required for go-ipfs and vice-versa ?
Not sure what you mean by pointless? :)
theobat
commented
7 years ago I mean 'not used'
Le 7 nov. 2016 8:17 AM, "Haad" notifications@github.com a écrit :
Great to hear!
Will there be some pointless configs for js-ipfs that are required for go-ipfs and vice-versa ?
Not sure what you mean by pointless? :)
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/haadcode/orbit/issues/166#issuecomment-258763105, or mute the thread https://github.com/notifications/unsubscribe-auth/AH15F3cVRHT540XO6007Ug_AOWo5gmbLks5q7tCLgaJpZM4KawWx .
haadcode
commented
7 years ago You mean if there's options/config params that are specific to either js-ipfs or go-ipfs?
There's one that I can think of: Signal Server Address for js-ipfs (this is not documented anywhere atm., it's a url that we should allow the user to change) that is not needed in go-ipfs.
jbenet
commented
7 years ago 
The user should be able to set the IPFS data folder, API/gateway/daemon addresses, etc. directly in the UI.
Proposal here is to add a button to the login screen which then opens a configuration screen where the user can change the configuration details. By default, the values should be the default values Orbit sets for the daemon.