Changed option is ignored

[gdkrmr]

Checklist

• This is a bug report, not a question. Ask questions on discuss.ipfs.io.
• I have searched on the issue tracker for my bug.
• I am running the latest kubo version or have an issue updating.

Installation method

ipfs-desktop

Version

This was on the docker version. 

Kubo version: 0.24.0-e70db65
Repo version: 15
System version: amd64/linux
Golang version: go1.21.4

Config

No response

Description

Changing the config

ipfs config --json Experimental.FilestoreEnabled true

and then running (without restarting, as in the documentation)

ipfs add --nocopy ...

will ignore the --nocopy and add everything as blocks. This should probably give an error.

[lidel]

Thank you for surfacing this UX problem @gdkrmr.
Yes, this looks like a bug – add --nocopy should error.

I think this is something not limited to FilestoreEnabled,
we have many flags that don't take effect without restarting, such as entire Routing, including Routing.AcceleratedDHTClient.

Fix proposal

This is a good opportunity to improve UX around config changes.

• Create a global config Flag named RequiresRestart and setting it to true every time when config change is applied.
  • Have an ignore list of config fields that don't require restart.
    • Why? Because it is shorter, and we won't miss new cofing keys added in the future. Default is to require restart, unless your command does not require it.
    • 👉 From the top of my head (we can expand later), configs that work without restart are limited to:
      • Pinning.RemoteServices
      • ...is there anything else?
  • Reset the flag during ipfs daemon.
• Then, check for flag in sensitive commands like ipfs add --nocopy
  • If the RequiresRestart is set to true, error asking user to restart first.
  • 👉 👉 From the top of my head (we can expand later), we should check the flag the beginning of all "mutable" and "publish" commands such as:
    • ipfs add, ipfs block put, ipfs dag put|import, ipfs files mkdir|write – we will add ability to adjust default CID parmeters in the future, so config impact won't be limited to add --nocopy
    • ipfs routing provide (impacted by AcceleratedDHTClient)
    • ipfs name publish (impacted by IPNS config params)`
• There should also be a periodical check that also warns users who should restart, like we did in feat: warn users who are falling behind reprovides #9886

[Jonath-z]

@gdkrmr can look into this? First time contributing to open source ;-), I would love just more guidance to work on this