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

config: defaults difficult to update #8776

Open
aschmahmann opened this issue Oct 17, 2019 · 3 comments
Open

config: defaults difficult to update #8776

aschmahmann opened this issue Oct 17, 2019 · 3 comments
Labels
need/triage Needs initial labeling and prioritization topic/config Topic config

Comments

@aschmahmann
Copy link
Contributor

As it currently stands it's pretty difficult to change default behavior of clients since the bulk of the configuration is done during the initialization of the repo. A few examples:

  1. Want to have a value default to true, but because we use an empty go struct to define the config it defaults to false (forces us to have config names like DisableSigning instead of EnableSigning. (e.g. Enable pubsub by default #6621 (comment))

  2. Want to remove old bootstrap peers and replace them with new ones

  3. Want to change the default pubsub router from floodsub to gossipsub in the same place all the configs are defined (instead of defining the default as empty string "", and changing ipfs constructor code to reinterpret "" as gossipsub instead of floodsub.

Idea: Let's define defaults programmatically, and use the config files to add user preferences.

Defaults being defined programmatically solves problem 1 and makes defining default values easy (solves part of problem 3). Solving problem 2 and the rest of problem 3 is more complicated though.

A few options to consider:

  1. Create a default Config struct that we use as a base when reading in config files, and continue to write the entire config file to disk.

    • Advantages: Pretty easy to implement
    • Disadvantages: Does not help when we want to update configuration values for clients (e.g. problems 2 and 3)
  2. Whenever we write a config file, cfg to disk calculate the difference cfg - defaultCfg and write that instead.

    • Advantages: It keeps the config size minimal and makes it easy to upgrade users who already have old config files. Solves problems 1,2,3.
    • Disadvantages: Users' config files could potentially get modified without them realizing it. Additionally, users may want to retain the old defaults automatically even when we change (e.g. stick with floodsub instead of moving to gossipsub).
  3. Create a copy-on-write style config object where the net config object is the defaults + what's read from disk

    • Advantages: Keeps config size limited to what people care about (excluding existing users who will have to decide if they want to blow away their configs or not). Solves problems 1,2,3.
    • Disadvantages: Requires a large style change by moving to a Getter/Setter model from a struct field model. Does not by itself draft existing users into upgrading their config files, if that's something we want.

My plan is to start on option 1 because it gives us the most utility for the least work. However, I'd like to talk about options 2 and 3.

Thoughts @Stebalien @magik6k

@daviddias
Copy link
Member

Ideally, this would be in sync with js-ipfs. In js-ipfs there is a sequence in which the final config is set. It is done by:

cli args (if through daemon) > env variables > options passed on node constructor > new config passed on the constructor > default config.json

The new config passed on the constructor is merged with the default config (so you only need to pass the values you want to change/augment) https://github.com/ipfs/js-ipfs#optionsconfig

@aschmahmann
Copy link
Contributor Author

@daviddias how does that deal with IPFS CLI commands that set one key in the config file? go-ipfs loads the config file, sets the key, then writes the entire config (which could include default options) back. This approach makes it difficult to deal with updating defaults.

@Stebalien
Copy link
Member

I'd go with 3, actually, and I don't think it'll be a problem.

  1. Reading the config isn't an issue. The "config" object we expose can include the defaults.
  2. When writing the config, we usually call special setter functions anyways.

@lidel lidel changed the title Defaults difficult to update config: defaults difficult to update Mar 9, 2022
@lidel lidel transferred this issue from ipfs/go-ipfs-config Mar 9, 2022
@lidel lidel added need/triage Needs initial labeling and prioritization topic/config Topic config labels Mar 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization topic/config Topic config
Projects
None yet
Development

No branches or pull requests

4 participants