-
Notifications
You must be signed in to change notification settings - Fork 21
CNDB-13624: Silence snakeyaml warnings in test-cdc by removing duplicate cdc_enabled key in base yaml #1680
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
Conversation
…ate cdc_enabled key in base yaml
Checklist before you submit for review
|
|
✔️ Build ds-cassandra-pr-gate/PR-1680 approved by ButlerApproved by Butler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
commitlog_compression also exists in the base yaml, but it is commented out so it does not produce any warnings.
This new version logs when duplicate keys are found.
It was high time, this was a reason for confusion and some bugs. Half people actively use it, half people have no clue overriding exists. That is how CASSANDRA-17379 was born. From Apache Cassandra 4.1+:
[CASSANDRA-17379](https://issues.apache.org/jira/browse/CASSANDRA-17379) was opened to improve the user experience and deprecate the overloading. By default, we refuse starting Cassandra with a config containing both old and new config keys for the same parameter. Start Cassandra with -Dcassandra.allow_new_old_config_keys=true to override. For historical reasons duplicate config keys in cassandra.yaml are allowed by default, start Cassandra with -Dcassandra.allow_duplicate_config_keys=false to disallow this. Please note that key_cache_save_period, row_cache_save_period, counter_cache_save_period will be affected only by -Dcassandra.allow_duplicate_config_keys.
|
Thanks for the review! I briefly considered disabling these warnings to revert to the old behavior, but I think they're a positive change. |
…ate cdc_enabled key in base yaml (#1680) ### What is the issue #1563 upgraded our version of snakeyaml. This new version logs when duplicate keys are found. The test-cdc target concatenates a CDC-specific yaml to the base yaml. Both contain entries for the cdc_enabled key, causing tests that load this yaml to log the warning. This causes CDC CI failures. ### What does this PR fix and why was it fixed This PR removes the cdc_enabled entry in the base yaml, as it's the same as the default. This is consistent with how we handle other default keys that might be duplicated across multiple yaml fragments in the test commands.
…ate cdc_enabled key in base yaml (#1680) ### What is the issue #1563 upgraded our version of snakeyaml. This new version logs when duplicate keys are found. The test-cdc target concatenates a CDC-specific yaml to the base yaml. Both contain entries for the cdc_enabled key, causing tests that load this yaml to log the warning. This causes CDC CI failures. ### What does this PR fix and why was it fixed This PR removes the cdc_enabled entry in the base yaml, as it's the same as the default. This is consistent with how we handle other default keys that might be duplicated across multiple yaml fragments in the test commands.



What is the issue
#1563 upgraded our version of snakeyaml. This new version logs when duplicate keys are found. The test-cdc target concatenates a CDC-specific yaml to the base yaml. Both contain entries for the cdc_enabled key, causing tests that load this yaml to log the warning. This causes CDC CI failures.
What does this PR fix and why was it fixed
This PR removes the cdc_enabled entry in the base yaml, as it's the same as the default. This is consistent with how we handle other default keys that might be duplicated across multiple yaml fragments in the test commands.