ipfs / kubo

An IPFS implementation in Go
https://docs.ipfs.tech/how-to/command-line-quick-start/
Other
16.13k stars 3.01k forks source link

changes in the default configuration do not propagate to the upgrades of earlier go-ipfs versions #3707

Open Mithgol opened 7 years ago

Mithgol commented 7 years ago

Version information

go-ipfs version: 0.4.5-
Repo version: 5
System version: amd64/windows
Golang version: go1.7.4

Type: bug (misconfiguration)

Priority: ?

Description

The chat log from fedwiki/wiki-client#173 says that changes in the default configuration of go-ipfs do not automatically apply to users of earlier versions when they upgrade. Click to enlarge:

(the chat log)

It means that earlier adopters do not automatically get config-related features (such as CORS from #2778) and it makes those features less reliable.

For example, I cannot reliably host a 360°×180° photo for a Pannellum-based panorama on IPFS (unless I decide to achieve “same origin” by putting the whole engine of Panellum on IPFS as well, but that would be quite a 無駄) because:

Something should be done about it. Consider, for example, two separate configuration files (one for the default config and the other for the overrides of the defaults).

Kubuxu commented 7 years ago

Thanks for rising it. I think this will be becoming more and more of an issue as we expand and want to move config forward.

I agree that user config and separate default config might be a solution for that. This would mean refactor of configuration system and possible migrations code.

I would also like to raise the issue of modifying config using the ipfs tool itself. It is problematic, not really clean (you have to restart to see the effect which isn't clear at first) and it will prevent introduction of better config format (yaml or other more human oriented format) as it requires re-serialization of config file which will get messy.

I can see similarity to git config CLI-API but we have to ask ourselves if that works for us. git doesn't have a daemon, git stores mostly user 'preferences' and not application configuration inside the config file.

From other side I know that API stability is very important which means that it would be best not to interfere with it.

@whyrusleeping @jbenet as you were who created this API first I would like to hear your thoughts on this matter over all.

djdv commented 7 years ago

I was wondering about this too with the recent ipv6 additions (https://github.com/ipfs/go-ipfs/pull/3523), I wouldn't have added them to my config if I didn't notice it. Adding my 2 cents, I've always been a fan of a magic default value (such as "default") with accompanying documentation on what that expands into and maybe notes for version changes "in v1 X defaulted to Y, in v2 this was changed to Z".

i.e. if you don't want to maintain your own list of peers and would rather just use whatever the defaults are now and in the future, you could just say "Bootstrap":"default" or something like that in your config file.

Omitting them is another thing I'm a fan of, if there's nothing user defined, assert the default for that parameter. i.e. if "Type" is missing inside Datastore{}, assume "leveldb" (current default) I like this because then you don't end up with a config full of deprecated options that used to be there and don't have to manually add new things as they're added either.

Either of these would be nice for passively adapting to upstream config changes without interfering with those who want to alter the config.

Yet another option would be something like ipfs config upgrade|scan|check|etc. that handles whatever needs to be handled, like inserting/reporting new options, removing/reporting deprecated options, reporting value errors, etc. After a version update people could check their config with it to see if any action is necessary/recommended.

whyrusleeping commented 7 years ago

I think one thing we can do pretty easily is add an ipfs config upgrade command that handles doing these changes. I'm not certain that having a magic keyword of default is the right thing to do, but i'm open to discussing it further.

the config upgrade command could work at least two different ways, the first would be for us to manually add in changes we make to the command. This becomes cumbersome to keep updated and builds up in un-fun ways over time (we could put it in an fs-repo-migration?).

The second way would be to generate a new default config, and diff it against the currently existing one, prompting the user for whether or not they want to update each hunk (to avoid overwriting user modified parts).

Mithgol commented 7 years ago

Having two separate configuration files (one for the default config and the other for the overrides of the defaults; the former might be internal to the executable) seems to require less knowledge and less efforts from end users than that ipfs config upgrade requires.

Kubuxu commented 7 years ago

I am also for having config overrides. My proposal is: $IPFS_PATH/cfg/ directory containing overrides for default configuration (files are loaded in alphanumeric order).

The current default configuration could be displayed with ipfs config default and current config with overrides applied under ipfs config show.

The current API of ipfs config Path Value would work by having specific file in the configuration override directory (like 50-cli-overrides.json).

whyrusleeping commented 7 years ago

Yeah, we should add this to the migrations code. Or at least add a way to say "reset config defaults", maybe ipfs config reset ?

lidel commented 4 years ago

For what its worth, the 0.6 release (https://github.com/ipfs/go-ipfs/issues/7347) contains a small config migration to enable QUIC by default.