Rename get_default_config to get_default_metrics in redis_cloud/redis_enterprise checks - #3128
Closed
philjlee wants to merge 1 commit into
Conversation
…_enterprise checks Both RedisCloudCheck and RedisEnterpriseCheck defined a get_default_config() method returning a list of metric groups. A newer datadog-checks-base adds its own get_default_config() to OpenMetricsBaseCheckV2, used internally by get_config_with_defaults() to seed scraper config from the generated config_models defaults (expects key/value pairs, not a metrics list). The override shadows the base method, so get_config_with_defaults() calls the check's version instead and dict() blows up trying to consume a list of metric dicts as key/value pairs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
philjlee
changed the base branch from
master
to
philip.lee/fix-validate-config-models-drift
August 24, 2026 20:57
This comment has been minimized.
This comment has been minimized.
philjlee
force-pushed
the
philip.lee/fix-redis-openmetrics-default-config-collision
branch
from
August 24, 2026 21:04
3420457 to
0ce0222
Compare
Collaborator
Author
|
Superseded — combined into #3127. Both PRs touched the same redis_cloud/redis_enterprise directories, and under CI's TARGET=changed scoping neither could pass independently of the other (whichever was checked standalone would fail on the other's pre-existing bug). Folded the get_default_config rename into #3127 alongside the drift fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Renames the
get_default_config()method onRedisCloudCheckandRedisEnterpriseChecktoget_default_metrics().Why
Both checks defined their own
get_default_config()returning a list ofmetric groups. A newer
datadog-checks-baseadds its ownget_default_config()hook toOpenMetricsBaseCheckV2, used internally byget_config_with_defaults()to seed scraper config from the check'sgenerated
config_modelsdefaults (expects key/value pairs). Becausepyproject.tomlpinsdatadog-checks-base>=37.20.0with no upper bound,CI resolves the latest version, whose base class method gets shadowed by
each check's same-named override —
get_config_with_defaults()then callsthe check's list-returning method instead and
dict()raises:Confirmed pre-existing and unrelated to any other open PR: reproduced on an
unmodified checkout of
masterviauvx --python 3.13 ddev@18.0.0 -x test redis_cloud— the failure is byte-for-byte identical. It's only visible inCI when a PR happens to touch files inside
redis_cloud/orredis_enterprise/, sinceTARGET=changedscopes the test matrix totouched directories.
How
Renamed the check-specific method (and its one call site in
_parse_config) in both integrations fromget_default_configtoget_default_metrics, which no longer collides with the base class'sconfig-defaults hook.
Verified locally with
ddev@18.0.0(the version CI'stestworkflowresolves):
redis_cloud5/5 passing,redis_enterprise7/7 passing, lintclean.