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

[MM-57106] Remove unused cluster settings #26490

Merged
merged 9 commits into from
Mar 22, 2024

Conversation

hanzei
Copy link
Contributor

@hanzei hanzei commented Mar 15, 2024

Summary

The settings StreamingPort, MaxIdleConns, MaxIdleConnsPerHost and IdleConnTimeoutMilliseconds were unused and had no effect. This PR cleans them up from the codebase.

Ticket Link

https://mattermost.atlassian.net/browse/MM-57106

Release Note

Remove unused cluster settings: `StreamingPort`, `MaxIdleConns`, `MaxIdleConnsPerHost`, `IdleConnTimeoutMillise.conds`

@mm-cloud-bot mm-cloud-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 15, 2024
@@ -33,7 +33,7 @@ func TestUpdateConfigRace(t *testing.T) {

cfg := &model.Config{}
cfg.SetDefaults()
cfg.ClusterSettings.MaxIdleConns = model.NewInt(1)
cfg.ClusterSettings.GossipPort = model.NewInt(9999)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needed another setting for the test

@hanzei hanzei added 2: Dev Review Requires review by a developer Docs/Needed Requires documentation Changelog/Done Required changelog entry has been written 3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review labels Mar 18, 2024
@mattermost-build
Copy link
Contributor

E2E tests not automatically triggered, because the PR is not in a mergeable state. Please update the branch with the base branch and resolve outstanding conflicts.

@hanzei hanzei marked this pull request as ready for review March 18, 2024 08:51
Copy link
Member

@agnivade agnivade left a comment

Choose a reason for hiding this comment

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

Feel good cleanup!

Copy link
Contributor

@streamer45 streamer45 left a comment

Choose a reason for hiding this comment

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

Cool 👍

@streamer45 streamer45 removed the 2: Dev Review Requires review by a developer label Mar 18, 2024
@hanzei
Copy link
Contributor Author

hanzei commented Mar 22, 2024

/e2e-test

@mattermost-build
Copy link
Contributor

Successfully triggered E2E testing!
GitLab pipeline | Test dashboard

@hanzei hanzei added this to the v9.8.0 milestone Mar 22, 2024
@hanzei
Copy link
Contributor Author

hanzei commented Mar 22, 2024

E2E tests look good. Merging.

@hanzei hanzei merged commit 7337c38 into master Mar 22, 2024
48 checks passed
@hanzei hanzei deleted the MM-57106_cleanup-cluster-settings branch March 22, 2024 09:44
@hanzei
Copy link
Contributor Author

hanzei commented Mar 22, 2024

@cwarnermm Heads up that doc updates are required for this PR. Please let me know if you need anything from me. Should be straightforward to move the removed cluster settings to the legacy config page.

@jwilander jwilander added the kind/refactor Categorizes issue or PR as related to refactor of production code. label May 7, 2024
@amyblais amyblais added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3: QA Review Requires review by a QA tester. May occur at the same time as Dev Review Changelog/Done Required changelog entry has been written Docs/Done Required documentation has been written kind/refactor Categorizes issue or PR as related to refactor of production code. release-note Denotes a PR that will be considered when it comes time to generate release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants