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

Distinguish between Offline and Local Mode. #3259

Merged
merged 1 commit into from
Nov 2, 2016
Merged

Conversation

kevina
Copy link
Contributor

@kevina kevina commented Sep 25, 2016

The filestore code (#2634) does not care if the daemon is in online or offline mode, it cares if it is running or not. There is various tests throughout the code for this but no standardized way to get the information. The p.r. attempts to rectify that.

For the full context see b34fd9c.

License: MIT
Signed-off-by: Kevin Atkinson k@kevina.org

License: MIT
Signed-off-by: Kevin Atkinson <k@kevina.org>
@Kubuxu Kubuxu added the status/in-progress In progress label Sep 25, 2016
@kevina
Copy link
Contributor Author

kevina commented Sep 25, 2016

Please let me know if this is a good way to go about it.

@kevina kevina mentioned this pull request Sep 25, 2016
6 tasks
@whyrusleeping
Copy link
Member

Hrm... this is getting to be a bit complex. Let me think out loud here for a bit...

There are a few different 'modes' of an ipfs node:

  • A local ephemeral node
    • This is what is used when running cli commands without a daemon. It does not have network functionality, and does not make api requests to some 'remote' daemon, but executes everything locally. This mode is currently distinct from running a command with --local, but maybe that should be pushed as a more respected flag.
  • A 'permanent' 'online' daemon
    • This is a node constructed to be run as a daemon process, with networking and an API service. The implication of the api service here is that the daemon and the client may not necessarily be on the same machine.
  • A 'permanent' 'offline' daemon
    • This is the same as the previous, except with the caveat that no peer to peer networking will be enabled.

There are potentially more permutations, but for now i think those are all that really matter here...

We need to be able to tell if the node is networked or not to restrict certain commands, we need online/offline. To tell if we should start certain 'long running' daemon processes, we need to know if this node was started with the intention of being a daemon or not. To tell if we are acting on a node where the disk storage is the same for client and daemon (a fairly specific use-case) we need to know local vs remote. I think that we can probably continue to do it the way we have (with the constants representing various degrees of node-state), but we probably want to clean it up a bit soon. And having helper methods for each nodestate is probably acceptable, given that the number of options is finite and small and also that the setter for them all is simply just SetMode(mode).

So that all said, I think for this changeset we should try to use SetMode instead of adding an extra helper for SetLocal.

@kevina
Copy link
Contributor Author

kevina commented Sep 26, 2016

@whyrusleeping, my main concern is if I am setting the mode to localMode correctly, to be honest I am not 100% sure I got all the cases. Do you think it would be a good idea to replace the checks we have if the daemon is running with a check for IsLocal?

@whyrusleeping
Copy link
Member

Do you think it would be a good idea to replace the checks we have if the daemon is running with a check for IsLocal?

I would need to think through the implications of that a bit more. The primary weird cases that come to mind is running a command with --local while a daemon is running, and also when running a daemon with the --offline flag. With --offline, 'offline' and 'local' no longer mean the same thing.

@kevina
Copy link
Contributor Author

kevina commented Sep 26, 2016

Okay than. I care about two things: 1) Will the commands be executed locally, or via the API, and 2) Can I assume I have an exclusive lock on the repo.

@whyrusleeping
Copy link
Member

@kevina I think that its safe to assume that if the node is 'local', then you have that exclusive lock.

@whyrusleeping
Copy link
Member

So really what we want to know here is "is this command being run over the http API?" right?

If thats the case, we can get that information much more easily through the commands invocation context. (if it currently doesnt provide that to us, it would be really easy to add)

@kevina
Copy link
Contributor Author

kevina commented Oct 6, 2016

@whyrusleeping, if the command not being executed over the http API also means I have an exclusive lock on the repo, then Yes, that is all I need to know.

@kevina kevina added this to the Filestore implementation milestone Oct 19, 2016
@whyrusleeping
Copy link
Member

Related to #3347

We can probably merge this as is (assuming it works for you @kevina ) and then @Kubuxu can refactor this all later when resolving #3347

@whyrusleeping whyrusleeping added status/ready Ready to be worked and removed status/in-progress In progress labels Nov 2, 2016
@kevina
Copy link
Contributor Author

kevina commented Nov 2, 2016

@whyrusleeping It works for me. Merge when every your ready.

@kevina kevina changed the title RFC: Distinguish between Offline and Local Mode. Distinguish between Offline and Local Mode. Nov 2, 2016
@whyrusleeping whyrusleeping merged commit 4b793f9 into master Nov 2, 2016
@whyrusleeping whyrusleeping deleted the kevina/local-mode branch November 2, 2016 21:04
@whyrusleeping whyrusleeping removed the status/ready Ready to be worked label Nov 2, 2016
@ghost ghost mentioned this pull request Dec 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants