Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

refactor(config): remove Swarm.ConnMgr defaults #8913

Merged

Conversation

schomatis
Copy link
Contributor

Fixes #8845.

Sorry for the delay. I was digging around config's Optional structs implementation to look for a cleaner approach but that shouldn't be blocking this (it can be addressed later in a separate issue).

@schomatis schomatis requested a review from lidel April 26, 2022 23:51
@schomatis schomatis self-assigned this Apr 26, 2022
@schomatis

This comment was marked as resolved.

@schomatis schomatis force-pushed the schomatis/chore/config/optional-types-for-Swarm.ConnMgr branch from 0b7809a to c1c92db Compare April 27, 2022 14:23
@schomatis schomatis enabled auto-merge April 27, 2022 14:42
@lidel lidel force-pushed the schomatis/chore/config/optional-types-for-Swarm.ConnMgr branch 2 times, most recently from ec4b83f to 22ece13 Compare November 10, 2022 23:51
@lidel lidel changed the title chore(config): remove Swarm.ConnMgr defaults from user config refactor(config): remove Swarm.ConnMgr defaults Nov 10, 2022
@lidel lidel force-pushed the schomatis/chore/config/optional-types-for-Swarm.ConnMgr branch from 22ece13 to a47c4a4 Compare November 11, 2022 00:05
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've rebased this and updated docs. Lgtm.

  • This PR enables us to change implicit defaults without having to modify user config.
  • One-time config migration may be useful, to remove historical defaults from user configs, but that would be a separate PR, where we also lower the defaults (no need to keep hundreds of connections around anymore, its a relict from the time DHT was not performing well).

Let's park this until next week – if there is 0.17.0-rc2, we would like to include this. If not, it can wait until 0.18.

@BigLep BigLep mentioned this pull request Nov 15, 2022
Copy link
Contributor

@BigLep BigLep left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per 2022-11-15 standup, the change conceptually makes sense and sounds good.

We can remove the rcmgr_defaults.go change due to recent changes in #9407

This moves defaults to Kubo code, cleaning up config.
If value is in config, we assume it is an explicit choice made by user.
Makes migrations easier.
@lidel lidel force-pushed the schomatis/chore/config/optional-types-for-Swarm.ConnMgr branch from a47c4a4 to d2f3fd9 Compare November 15, 2022 18:57
@lidel lidel disabled auto-merge November 15, 2022 19:05
@lidel lidel merged commit e34c0da into master Nov 15, 2022
@lidel lidel deleted the schomatis/chore/config/optional-types-for-Swarm.ConnMgr branch November 15, 2022 19:27
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Improve defaults in Swarm.ConnMgr
3 participants