refactor: Remove the StandaloneKafkaConfig struct #4253
Conversation
|
Warning Rate limit exceeded@irenjj has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 37 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes primarily involve renaming the Changes
Sequence Diagram(s)No sequence diagrams are necessary for these changes as they primarily involve renaming and refactoring configuration structs and fields. Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
src/common/wal/src/config.rs (1)
Line range hint
191-216: Duplicate deserialization test forDatanodeWalConfig.The deserialization test for
DatanodeWalConfigis duplicated. Remove the redundant test.- // Deserialized to DatanodeWalConfig. - let standalone_wal_config: DatanodeWalConfig = toml::from_str(toml_str).unwrap(); - let expected = DatanodeKafkaConfig { - broker_endpoints: vec!["127.0.0.1:9092".to_string()], - num_topics: 32, - selector_type: TopicSelectorType::RoundRobin, - topic_name_prefix: "greptimedb_wal_topic".to_string(), - num_partitions: 1, - replication_factor: 1, - create_topic_timeout: Duration::from_secs(30), - compression: Compression::default(), - max_batch_size: ReadableSize::mb(1), - linger: Duration::from_millis(200), - consumer_wait_timeout: Duration::from_millis(100), - backoff: BackoffConfig { - init: Duration::from_millis(500), - max: Duration::from_secs(10), - base: 2, - deadline: Some(Duration::from_secs(60 * 5)), - }, - }; - assert_eq!(standalone_wal_config, DatanodeWalConfig::Kafka(expected));
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/cmd/src/standalone.rs (4 hunks)
- src/cmd/tests/load_config_test.rs (2 hunks)
- src/common/wal/src/config.rs (7 hunks)
- src/common/wal/src/config/kafka.rs (1 hunks)
- src/common/wal/src/config/kafka/datanode.rs (3 hunks)
Files skipped from review due to trivial changes (1)
- src/common/wal/src/config/kafka.rs
Additional comments not posted (10)
src/common/wal/src/config/kafka/datanode.rs (3)
22-22: Ensure theTopicSelectorTypeand constants are correctly defined.The line imports
TopicSelectorType,BROKER_ENDPOINT, andTOPIC_NAME_PREFIX. Verify that these are correctly defined in the respective module.Verification successful
The
TopicSelectorType,BROKER_ENDPOINT, andTOPIC_NAME_PREFIXare correctly defined and imported.
TopicSelectorTypeis defined as an enum insrc/common/wal/src/lib.rs.BROKER_ENDPOINTis defined as a constant insrc/common/wal/src/lib.rs.TOPIC_NAME_PREFIXis defined as a constant insrc/common/wal/src/lib.rs.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition of TopicSelectorType, BROKER_ENDPOINT, and TOPIC_NAME_PREFIX. # Test: Search for the definitions. Expect: Definitions in the codebase. rg --type rust $'TopicSelectorType' rg --type rust $'const BROKER_ENDPOINT' rg --type rust $'const TOPIC_NAME_PREFIX'Length of output: 1422
44-56: New fields added toDatanodeKafkaConfig.Ensure that the new fields (
num_topics,num_partitions,selector_type,replication_factor,create_topic_timeout,topic_name_prefix) are correctly integrated and used within the codebase.Verification successful
New fields added to
DatanodeKafkaConfigare correctly integrated and used within the codebase.The new fields (
num_topics,num_partitions,selector_type,replication_factor,create_topic_timeout,topic_name_prefix) are found in various parts of the codebase, indicating proper integration and usage.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new fields in DatanodeKafkaConfig. # Test: Search for the new fields. Expect: Usage in the codebase. rg --type rust $'num_topics' rg --type rust $'num_partitions' rg --type rust $'selector_type' rg --type rust $'replication_factor' rg --type rust $'create_topic_timeout' rg --type rust $'topic_name_prefix'Length of output: 12394
71-76: Default values for new fields inDatanodeKafkaConfig.Ensure that the default values for the new fields (
num_topics,num_partitions,selector_type,replication_factor,create_topic_timeout,topic_name_prefix) are appropriate and consistent with the expected configuration.Verification successful
Default values for new fields in
DatanodeKafkaConfigverified.The default values for the new fields (
num_topics,num_partitions,selector_type,replication_factor,create_topic_timeout,topic_name_prefix) are appropriate and consistent with the expected configuration.
num_topics: 64num_partitions: 1selector_type: TopicSelectorType::RoundRobinreplication_factor: 1create_topic_timeout: Duration::from_secs(30)topic_name_prefix: TOPIC_NAME_PREFIX.to_string()Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the default values of new fields in DatanodeKafkaConfig. # Test: Search for the default values. Expect: Consistent usage in the codebase. rg --type rust $'num_topics: 64' rg --type rust $'num_partitions: 1' rg --type rust $'selector_type: TopicSelectorType::RoundRobin' rg --type rust $'replication_factor: 1' rg --type rust $'create_topic_timeout: Duration::from_secs(30)' rg --type rust $'topic_name_prefix: TOPIC_NAME_PREFIX.to_string()'Length of output: 1436
Script:
#!/bin/bash # Description: Verify the initialization of `replication_factor` and `create_topic_timeout` in the codebase. # Test: Search for the initialization of `replication_factor` and `create_topic_timeout`. Expect: Consistent usage in the codebase. rg --type rust 'replication_factor' rg --type rust 'create_topic_timeout'Length of output: 2277
src/common/wal/src/config.rs (3)
20-20: Ensure theDatanodeKafkaConfigandMetasrvKafkaConfigimports are correct.The line imports
DatanodeKafkaConfigandMetasrvKafkaConfig. Verify that these are correctly defined in the respective module.Verification successful
The imports of
DatanodeKafkaConfigandMetasrvKafkaConfigare correct.
DatanodeKafkaConfigis defined insrc/common/wal/src/config/kafka/datanode.rs.MetasrvKafkaConfigis defined insrc/common/wal/src/config/kafka/metasrv.rs.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition of DatanodeKafkaConfig and MetasrvKafkaConfig. # Test: Search for the definitions. Expect: Definitions in the codebase. rg --type rust $'pub struct DatanodeKafkaConfig' rg --type rust $'pub struct MetasrvKafkaConfig'Length of output: 250
Line range hint
46-56: Conversion fromDatanodeWalConfigtoMetasrvWalConfig.Ensure that the conversion logic from
DatanodeWalConfigtoMetasrvWalConfigis correct and handles all fields appropriately.Verification successful
Verification successful: The conversion logic from
DatanodeWalConfigtoMetasrvWalConfighandles all fields appropriately.
- All fields (
broker_endpoints,num_topics,selector_type,topic_name_prefix,num_partitions,replication_factor,create_topic_timeout, andbackoff) are correctly handled in the conversion.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the conversion logic from DatanodeWalConfig to MetasrvWalConfig. # Test: Search for the conversion implementation. Expect: Correct handling of all fields. rg --type rust $'impl From<DatanodeWalConfig> for MetasrvWalConfig'Length of output: 149
Script:
#!/bin/bash # Description: Extract the complete implementation of the `From<DatanodeWalConfig> for MetasrvWalConfig` trait. # Search for the complete implementation and display surrounding lines for full context. rg --type rust -A 20 -B 5 'impl From<DatanodeWalConfig> for MetasrvWalConfig'Length of output: 1798
Line range hint
64-74: Conversion fromMetasrvWalConfigtoDatanodeWalConfig.Ensure that the conversion logic from
MetasrvWalConfigtoDatanodeWalConfigis correct and handles all fields appropriately.Verification successful
Conversion logic is correct. The conversion from
MetasrvWalConfigtoDatanodeWalConfigappropriately handles all fields.
broker_endpointsnum_topicsselector_typetopic_name_prefixnum_partitionsreplication_factorcreate_topic_timeoutbackoffScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the conversion logic from MetasrvWalConfig to DatanodeWalConfig. # Test: Search for the conversion implementation. Expect: Correct handling of all fields. rg --type rust $'impl From<MetasrvWalConfig> for DatanodeWalConfig'Length of output: 149
Script:
#!/bin/bash # Extract the full implementation of the From trait for MetasrvWalConfig to DatanodeWalConfig ast-grep --lang rust --pattern $'impl From<MetasrvWalConfig> for DatanodeWalConfig { fn from(config: MetasrvWalConfig) -> Self { match config { MetasrvWalConfig::RaftEngine => $_, MetasrvWalConfig::Kafka($_) => $_, $$$ } } }'Length of output: 256
Script:
#!/bin/bash # Extract the implementation of the From trait for MetasrvWalConfig to DatanodeWalConfig and surrounding lines for context rg --type rust --context 10 'impl From<MetasrvWalConfig> for DatanodeWalConfig'Length of output: 1645
src/cmd/tests/load_config_test.rs (1)
23-23: Ensure theDatanodeWalConfigimport is correct.The line imports
DatanodeWalConfig. Verify that it is correctly defined in the respective module.src/cmd/src/standalone.rs (3)
42-42: Ensure theDatanodeWalConfigimport is correct.The line imports
DatanodeWalConfig. Verify that it is correctly defined in the respective module.Verification successful
The
DatanodeWalConfigimport is correct.The
DatanodeWalConfigis defined insrc/common/wal/src/config.rs, confirming the correctness of the import statement insrc/cmd/src/standalone.rs.
src/common/wal/src/config.rs:pub enum DatanodeWalConfig {Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the definition of DatanodeWalConfig. # Test: Search for the definition. Expect: Definition in the codebase. rg --type rust $'pub enum DatanodeWalConfig'Length of output: 103
129-129: Ensure thewalfield inStandaloneOptionsis correctly integrated.The line adds the
walfield toStandaloneOptions. Verify that it is correctly integrated and used within the codebase.
154-154: Ensure the default value for thewalfield inStandaloneOptionsis correct.The line sets the default value for the
walfield inStandaloneOptions. Verify that the default value is appropriate and consistent with the expected configuration.Verification successful
The default value for the
walfield inStandaloneOptionsis appropriate.The
walfield is set toDatanodeWalConfig::default(), which defaults toDatanodeWalConfig::RaftEngine(RaftEngineConfig::default()). The default values forRaftEngineConfiginclude reasonable settings such as a file size of 256 MB, a purge threshold of 4 GB, and other typical configurations for a Raft engine.
src/common/wal/src/config.rssrc/common/wal/src/config/raft_engine.rsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the default value of the wal field in StandaloneOptions. # Test: Search for the default value of the wal field. Expect: Consistent usage in the codebase. rg --type rust $'wal: DatanodeWalConfig::default()'Length of output: 282
Script:
#!/bin/bash # Locate the implementation of DatanodeWalConfig::default() to inspect the default values. ast-grep --lang rust --pattern 'impl DatanodeWalConfig { fn default() -> Self { $_ } }'Length of output: 105
Script:
#!/bin/bash # Locate the definition of the DatanodeWalConfig struct rg --type rust 'struct DatanodeWalConfig' # Once we have the file, locate the implementation of the default method for DatanodeWalConfig within the same file rg --type rust 'impl DatanodeWalConfig' -A 10Length of output: 91
Script:
#!/bin/bash # Search for the keyword 'DatanodeWalConfig' in the entire repository to locate its definition and related implementations. rg 'DatanodeWalConfig'Length of output: 5970
Script:
#!/bin/bash # Search for the impl Default for DatanodeWalConfig block in src/common/wal/src/config.rs to inspect the default values. rg 'impl Default for DatanodeWalConfig' -A 10 src/common/wal/src/config.rsLength of output: 474
Script:
#!/bin/bash # Search for the impl Default for RaftEngineConfig block to inspect the default values. rg 'impl Default for RaftEngineConfig' -A 10Length of output: 889
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/common/wal/src/config.rs (7 hunks)
- src/common/wal/src/config/kafka/datanode.rs (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/common/wal/src/config.rs
- src/common/wal/src/config/kafka/datanode.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/cmd/src/standalone.rs (4 hunks)
- src/cmd/tests/load_config_test.rs (2 hunks)
- src/common/wal/src/config.rs (7 hunks)
- src/common/wal/src/config/kafka/datanode.rs (2 hunks)
Files skipped from review as they are similar to previous changes (4)
- src/cmd/src/standalone.rs
- src/cmd/tests/load_config_test.rs
- src/common/wal/src/config.rs
- src/common/wal/src/config/kafka/datanode.rs
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4253 +/- ##
==========================================
- Coverage 85.18% 84.90% -0.29%
==========================================
Files 1060 1059 -1
Lines 189069 189033 -36
==========================================
- Hits 161057 160493 -564
- Misses 28012 28540 +528 |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/common/wal/src/config/kafka/datanode.rs (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/common/wal/src/config/kafka/datanode.rs
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs (5 hunks)
- src/common/wal/src/config.rs (5 hunks)
- src/common/wal/src/config/kafka/common.rs (2 hunks)
- src/common/wal/src/config/kafka/datanode.rs (3 hunks)
- src/common/wal/src/config/kafka/metasrv.rs (2 hunks)
Additional comments not posted (18)
src/common/wal/src/config/kafka/metasrv.rs (3)
19-19: LGTM! The import statement is necessary.The addition of
KafkaTopicConfig,backoff_prefix, andBackoffConfigis appropriate to support the new configuration structure.
31-31: LGTM! The addition of thekafka_topicfield improves modularity.The
kafka_topicfield of typeKafkaTopicConfigconsolidates topic-related configurations into a single struct.
38-49: LGTM! TheDefaultimplementation correctly initializes the new field.The
Defaultimplementation forMetasrvKafkaConfigcorrectly initializes thekafka_topicfield with appropriate default values.src/common/wal/src/config/kafka/datanode.rs (3)
21-21: LGTM! The import statement is necessary.The addition of
KafkaTopicConfig,backoff_prefix, andBackoffConfigis appropriate to support the new configuration structure.
43-45: LGTM! The addition of thekafka_topicfield improves modularity.The
kafka_topicfield of typeKafkaTopicConfigconsolidates topic-related configurations into a single struct.
57-57: LGTM! TheDefaultimplementation correctly initializes the new field.The
Defaultimplementation forDatanodeKafkaConfigcorrectly initializes thekafka_topicfield with appropriate default values.src/common/wal/src/config/kafka/common.rs (3)
20-21: LGTM! The import statement is necessary.The addition of
TopicSelectorTypeandTOPIC_NAME_PREFIXis appropriate to support the new configuration structure.
52-68: LGTM! The addition ofKafkaTopicConfigimproves modularity.The
KafkaTopicConfigstruct encapsulates topic-related configurations, improving modularity and readability.
70-81: LGTM! TheDefaultimplementation correctly initializes the fields.The
Defaultimplementation forKafkaTopicConfigcorrectly initializes the fields with appropriate default values.src/common/wal/src/config.rs (4)
20-20: LGTM! The import statement is necessary.The addition of
DatanodeKafkaConfigandMetasrvKafkaConfigis appropriate to support the new configuration structure.
46-53: LGTM! TheFromimplementation correctly maps the fields.The
Fromimplementation forDatanodeWalConfigtoMetasrvWalConfigcorrectly maps the fields, including the newkafka_topicfield.
59-66: LGTM! TheFromimplementation correctly maps the fields.The
Fromimplementation forMetasrvWalConfigtoDatanodeWalConfigcorrectly maps the fields, including the newkafka_topicfield.
Line range hint
82-187: LGTM! The test cases correctly reflect the changes.The test cases have been updated to reflect the new configuration structure, ensuring proper testing of the new mappings.
src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs (5)
59-60: LGTM! The initialization consolidates topic-related configurations.The initialization of the
topicsvector using thekafka_topicfield improves readability and maintainability.
63-63: LGTM! The initialization consolidates topic-related configurations.The initialization of the
selectorusing theselector_typefield fromkafka_topicimproves readability and maintainability.
79-79: LGTM! The usage consolidates topic-related configurations.The usage of the
num_topicsfield fromkafka_topicin thestartmethod improves readability and maintainability.
188-190: LGTM! The usage consolidates topic-related configurations.The usage of fields from
kafka_topicin thetry_create_topicmethod improves readability and maintainability.
Line range hint
245-293: LGTM! The test case correctly reflects the changes.The test case has been updated to reflect the new configuration structure, ensuring proper testing of the new mappings.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/common/wal/src/config.rs (4 hunks)
Additional comments not posted (5)
src/common/wal/src/config.rs (5)
20-20: Approved: Updated import statement.The import statement has been updated correctly to remove
StandaloneKafkaConfig.
46-53: Approved: Implementation ofFromtrait forDatanodeWalConfigtoMetasrvWalConfig.The conversion logic correctly maps the relevant fields from
DatanodeKafkaConfigtoMetasrvKafkaConfig.
59-66: Approved: Implementation ofFromtrait forMetasrvWalConfigtoDatanodeWalConfig.The conversion logic correctly maps the relevant fields from
MetasrvKafkaConfigtoDatanodeKafkaConfig.
79-79: Approved: Added import statement forKafkaTopicConfig.The import statement is necessary for the test cases that involve
KafkaTopicConfig.
159-166: Approved: Updated test cases forMetasrvKafkaConfigandDatanodeKafkaConfig.The test cases have been updated to correctly reflect the new structure, including the
kafka_topicfield.Also applies to: 183-192
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/common/meta/src/wal_options_allocator.rs (2 hunks)
Additional comments not posted (3)
src/common/meta/src/wal_options_allocator.rs (3)
126-126: Import statements are correct.The import of
KafkaTopicConfigis necessary for the test cases.
164-167: InitializeKafkaTopicConfigwith required fields.Ensure that all required fields for
KafkaTopicConfigare initialized.Are there any required fields that are missing in this initialization?
168-170: Configuration initialization is correct.The
MetasrvKafkaConfigis correctly initialized with thekafka_topicfield.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- tests-integration/src/tests/test_util.rs (3 hunks)
- tests-integration/tests/region_migration.rs (8 hunks)
Additional comments not posted (7)
tests-integration/src/tests/test_util.rs (2)
235-239: Verify the correctness of theKafkaTopicConfiginitialization.The new
KafkaTopicConfigis correctly initialized withtopic_name_prefix,num_topics, and default values for other fields. Ensure that these configurations are correct and complete.
268-272: Verify the correctness of theKafkaTopicConfiginitialization.The new
KafkaTopicConfigis correctly initialized withtopic_name_prefix,num_topics, and default values for other fields. Ensure that these configurations are correct and complete.tests-integration/tests/region_migration.rs (5)
122-126: Verify the correctness of theKafkaTopicConfiginitialization.The new
KafkaTopicConfigis correctly initialized withnum_topics,topic_name_prefix, and default values for other fields. Ensure that these configurations are correct and complete.
254-258: Verify the correctness of theKafkaTopicConfiginitialization.The new
KafkaTopicConfigis correctly initialized withnum_topics,topic_name_prefix, and default values for other fields. Ensure that these configurations are correct and complete.
379-383: Verify the correctness of theKafkaTopicConfiginitialization.The new
KafkaTopicConfigis correctly initialized withnum_topics,topic_name_prefix, and default values for other fields. Ensure that these configurations are correct and complete.
503-507: Verify the correctness of theKafkaTopicConfiginitialization.The new
KafkaTopicConfigis correctly initialized withnum_topics,topic_name_prefix, and default values for other fields. Ensure that these configurations are correct and complete.
642-646: Verify the correctness of theKafkaTopicConfiginitialization.The new
KafkaTopicConfigis correctly initialized withnum_topics,topic_name_prefix, and default values for other fields. Ensure that these configurations are correct and complete.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- config/config.md (1 hunks)
- config/metasrv.example.toml (1 hunks)
- src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs (5 hunks)
- src/common/wal/src/config.rs (4 hunks)
- src/common/wal/src/config/kafka/common.rs (2 hunks)
- src/common/wal/src/config/kafka/datanode.rs (3 hunks)
- src/common/wal/src/config/kafka/metasrv.rs (2 hunks)
- tests-integration/src/tests/test_util.rs (3 hunks)
- tests-integration/tests/region_migration.rs (8 hunks)
Files skipped from review due to trivial changes (1)
- config/config.md
Files skipped from review as they are similar to previous changes (6)
- src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs
- src/common/wal/src/config/kafka/common.rs
- src/common/wal/src/config/kafka/datanode.rs
- src/common/wal/src/config/kafka/metasrv.rs
- tests-integration/src/tests/test_util.rs
- tests-integration/tests/region_migration.rs
Additional comments not posted (12)
config/metasrv.example.toml (5)
80-80: LGTM!The renaming of
num_topicstokafka_topic_num_topicsis consistent with the new configuration structure.
85-85: LGTM!The renaming of
selector_typetokafka_topic_selector_typeis consistent with the new configuration structure.
88-88: LGTM!The renaming of
topic_name_prefixtokafka_topic_name_prefixis consistent with the new configuration structure.
91-91: LGTM!The renaming of
replication_factortokafka_topic_replication_factoris consistent with the new configuration structure.
94-94: LGTM!The renaming of
create_topic_timeouttokafka_topic_create_topic_timeoutis consistent with the new configuration structure.src/common/wal/src/config.rs (7)
20-20: LGTM!The import statement has been updated to include
DatanodeKafkaConfigandMetasrvKafkaConfig, consistent with the new configuration structure.
46-53: LGTM!The implementation of
From<DatanodeWalConfig> for MetasrvWalConfighas been updated to handleDatanodeWalConfiginstead ofStandaloneWalConfig, consistent with the new configuration structure.
59-66: LGTM!The implementation of
From<MetasrvWalConfig> for DatanodeWalConfighas been updated to handleMetasrvWalConfiginstead ofStandaloneWalConfig, consistent with the new configuration structure.
142-147: LGTM!The
test_toml_kafkatest case has been updated to reflect the new configuration structure, consistent with the renaming of configuration fields.
160-167: LGTM!The
KafkaTopicConfigstruct in thetest_toml_kafkatest case has been updated to reflect the new configuration structure, consistent with the renaming of configuration fields.
184-190: LGTM!The
KafkaTopicConfigstruct in thetest_toml_kafkatest case has been updated to reflect the new configuration structure forDatanodeWalConfig, consistent with the renaming of configuration fields.
193-193: LGTM!The assertion for
DatanodeWalConfigin thetest_toml_kafkatest case has been updated to reflect the new configuration structure, consistent with the renaming of configuration fields.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- config/config.md (1 hunks)
- tests/conf/metasrv-test.toml.template (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/conf/metasrv-test.toml.template
Files skipped from review as they are similar to previous changes (1)
- config/config.md
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/conf/metasrv-test.toml.template (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/conf/metasrv-test.toml.template
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/common/wal/src/config/kafka/common.rs (2 hunks)
- src/common/wal/src/config/kafka/metasrv.rs (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- src/common/wal/src/config/kafka/common.rs
- src/common/wal/src/config/kafka/metasrv.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- config/config.md (1 hunks)
- src/cmd/src/standalone.rs (4 hunks)
- src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs (5 hunks)
- src/common/wal/src/config.rs (4 hunks)
- src/common/wal/src/config/kafka/datanode.rs (3 hunks)
- tests-integration/fixtures/kafka/docker.json (1 hunks)
Files skipped from review due to trivial changes (1)
- src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs
Files skipped from review as they are similar to previous changes (4)
- config/config.md
- src/cmd/src/standalone.rs
- src/common/wal/src/config.rs
- src/common/wal/src/config/kafka/datanode.rs
Additional context used
Biome
tests-integration/fixtures/kafka/docker.json
[error] 1-1: Expected an array, an object, or a literal but instead found the end of the file.
Expected an array, an object, or a literal here.
(parse)
|
@WenyXu @fengjiachun Please take a look. |
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- config/config.md (1 hunks)
- config/metasrv.example.toml (1 hunks)
- src/cmd/src/standalone.rs (4 hunks)
- src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs (5 hunks)
- src/common/wal/src/config.rs (4 hunks)
- src/common/wal/src/config/kafka/common.rs (2 hunks)
- src/common/wal/src/config/kafka/datanode.rs (3 hunks)
- src/common/wal/src/config/kafka/metasrv.rs (1 hunks)
- tests/conf/metasrv-test.toml.template (1 hunks)
Files skipped from review as they are similar to previous changes (7)
- config/metasrv.example.toml
- src/common/meta/src/wal_options_allocator/kafka/topic_manager.rs
- src/common/wal/src/config.rs
- src/common/wal/src/config/kafka/common.rs
- src/common/wal/src/config/kafka/datanode.rs
- src/common/wal/src/config/kafka/metasrv.rs
- tests/conf/metasrv-test.toml.template
Additional context used
GitHub Check: Check typos and docs
config/config.md
[warning] 284-284:
"anme" should be "name" or "anime".
GitHub Check: Spell Check with Typos
config/config.md
[warning] 284-284:
"anme" should be "name" or "anime".
Additional comments not posted (4)
src/cmd/src/standalone.rs (4)
43-43: Import change approved.The import of
DatanodeWalConfigaligns with the PR objective of replacingStandaloneWalConfig.
133-133: Struct field change approved.The change from
StandaloneWalConfigtoDatanodeWalConfigaligns with the PR objective.
158-158: Default implementation change approved.The change from
StandaloneWalConfig::default()toDatanodeWalConfig::default()aligns with the PR objective.
207-207: Usage change approved but requires verification.The change from
StandaloneWalConfigtoDatanodeWalConfigis approved. However, ensure that all references towalare correctly integrated and used within the codebase.
[approved, verify]#!/bin/bash # Description: Verify the usage of the wal field in datanode_options. # Test: Search for the usage of the wal field. Expect: Correct integration in the codebase. rg --type rust $'wal: cloned_opts.wal'
|
PTAL @WenyXu @fengjiachun |
|
@irenjj Thanks! |
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
close: #4159
What's changed and what's your intention?
Remove the
StandaloneKafkaConfigstruct, and replaceStandaloneKafkaConfigwithDatanodeKafkaConfig.Checklist
Summary by CodeRabbit
Refactor
StandaloneWalConfigtoDatanodeWalConfig).Bug Fixes
wal.topic_name_prefix).New Features
KafkaTopicConfigstruct with detailed topic configuration options to enhance Kafka topic management.Documentation
Tests
KafkaTopicConfigstructure for better alignment with configuration changes.