Skip to content

Conversation

@bbrks
Copy link
Member

@bbrks bbrks commented Jan 30, 2026

CBG-5149

Stamp several default values into DefaultDbConfig() for visibility in config requests specifying include_runtime

  • Add test to catch future missed fields

Integration Tests

bbrks added 2 commits January 30, 2026 12:02
…they can be visible in the include_runtime config
…nclude_runtime - add test to catch future missed fields
Copilot AI review requested due to automatic review settings January 30, 2026 14:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses CBG-5149 by ensuring that default configuration values are properly stamped into DefaultDbConfig() so they are visible when users request configuration with the include_runtime parameter. The changes improve configuration transparency and add safeguards against future regressions.

Changes:

  • Adds explicit default values to several previously unset fields in DefaultDbConfig()
  • Introduces a comprehensive test that validates all DbConfig fields are either set to defaults or explicitly documented as intentionally unset
  • Documents the reasoning for fields that intentionally remain unset through an allowlist with explanatory comments

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
rest/config_database.go Stamps default values for SessionCookieName, DisablePasswordAuth, BucketOpTimeoutMs, Guest, ChangesRequestPlus, and ImportPartitions (CE case)
rest/config_database_test.go Adds TestDefaultDbConfigFieldCoverage to ensure all fields in DbConfig have either default values or documented reasons for being unset

@bbrks bbrks requested a review from RIT3shSapata January 30, 2026 14:09
@bbrks bbrks assigned RIT3shSapata and bbrks and unassigned RIT3shSapata Jan 30, 2026
@bbrks
Copy link
Member Author

bbrks commented Jan 30, 2026

@RIT3shSapata I'm not 100% sure but those failing integration tests might be relevant to this change, so I'll take a first-look before your review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants