Skip to content
This repository has been archived by the owner on Aug 22, 2022. It is now read-only.

Ability to configure the IPFS daemon in the UI #166

Closed
haadcode opened this issue Oct 19, 2016 · 13 comments
Closed

Ability to configure the IPFS daemon in the UI #166

haadcode opened this issue Oct 19, 2016 · 13 comments

Comments

@haadcode
Copy link
Member

haadcode commented Oct 19, 2016

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.

@theobat
Copy link
Contributor

theobat commented Oct 30, 2016

going to give this a shot

@haadcode
Copy link
Member Author

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
Copy link
Contributor

theobat commented Nov 2, 2016

Just a few architecture questions to follow up :

  • First of all, the default config object for the ipfs daemon has to move out of its current location, either a dedicated file (but where) or in the Login view as a const attribute. Perhaps echoing this comment we could have a dedicated folder with different default config files/objects for both ipfs and orbit itself ? what do you think ?
  • Secondly, 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 (twisting a little bit its original purpose ? not sure this is a good idea). Or perhaps somewhere else, I have no personal opinion on this as I never used reflux before.
  • Another thing is we could add the configuration of orbit paths itself in the same ui (this is less important but can help to think about the first point)

@haadcode
Copy link
Member Author

haadcode commented Nov 3, 2016

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
Copy link
Member Author

haadcode commented Nov 3, 2016

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
Copy link
Contributor

theobat commented Nov 3, 2016

I am not sure I understand this part of your post :

  1. 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):

  • first fire login view, user plays with it, when hit enters with a non null username :
  • then fire an action which inits the ipfs daemon (pretty much like you did before going to the LoginView in the index initially)
  • then emit the 'connect' event with the good username

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
Copy link
Contributor

theobat commented Nov 4, 2016

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
Copy link
Member Author

haadcode commented Nov 5, 2016

@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: ipfs/js-ipfs#556 (comment).

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
Copy link
Contributor

theobat commented Nov 6, 2016

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
Copy link
Member Author

haadcode commented Nov 7, 2016

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
Copy link
Contributor

theobat commented Nov 7, 2016

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
#166 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AH15F3cVRHT540XO6007Ug_AOWo5gmbLks5q7tCLgaJpZM4KawWx
.

@haadcode
Copy link
Member Author

haadcode commented Nov 7, 2016

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.

haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
@haadcode haadcode self-assigned this Dec 2, 2016
haadcode added a commit that referenced this issue Dec 2, 2016
Add the possibility to set Orbit's data directory in the configuration screen.
Make Orbit's data path isolated per user.
Polish configuration screen visuals.
Refactor startup sequence.
Add OrbitStore to the UI.
Remove obsolete code.
Fix build process and dist builds.
Remove obsolete scripts.

Closes #163.
Closes #166.
Closes #7.
Closes #167.
@jbenet
Copy link

jbenet commented Dec 4, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants