chore(kms-connector): simplify config parsing#1698
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the configuration parsing system across the kms-connector components by consolidating the separate raw.rs and parsed.rs modules into single config.rs files, using Serde's derive macros for direct deserialization with custom deserializers for complex types.
Key changes:
- Unified configuration parsing using Serde's
#[derive(Deserialize)]with custom deserializers forContractConfigandDurationtypes - Simplified
KmsWalletby making the enum public (removing the inner wrapper) - Standardized duration configurations to use
humantime_serdeformat - Changed monitoring endpoint default from
StringtoSocketAddrtype
Reviewed changes
Copilot reviewed 36 out of 42 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
kms-connector/crates/utils/src/config/ |
Removed raw.rs, added deserialize.rs trait; made contract module public; added PgInterval serialization helpers |
kms-connector/crates/utils/src/config/wallet.rs |
Changed KmsWallet from struct with private enum to public enum |
kms-connector/crates/utils/src/config/contract.rs |
Made RawContractConfig private; added custom deserializers and default functions for each contract type |
kms-connector/crates/tx-sender/src/core/config.rs |
Consolidated raw+parsed into single config with Serde derives; moved wallet building to build_wallet() method |
kms-connector/crates/kms-worker/src/core/config.rs |
Consolidated raw+parsed into single config with Serde derives |
kms-connector/crates/gw-listener/src/core/config.rs |
Consolidated raw+parsed into single config with Serde derives |
kms-connector/config/*.toml |
Updated comments to reflect humantime format; reorganized field order |
kms-connector/Cargo.toml |
Updated config (0.15.15→0.15.19), serde (1.0.226→1.0.228), toml (0.9.5→0.9.8); added humantime-serde; removed tempfile |
kms-connector/tests/ |
Removed entire integration test suite and Cargo.toml |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
🧪 CI InsightsHere's what we observed from your CI run for 6e070c0. 🟢 All jobs passed!But CI Insights is watching 👀 |
c3a002c to
6e070c0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 45 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Mergifyio queue |
✅ The pull request has been merged automaticallyDetailsThe pull request has been merged automatically at debc5ff |
Merge Queue Status✅ The pull request has been merged at 6e070c0 This pull request spent 39 minutes 6 seconds in the queue, including 43 minutes 42 seconds running CI. Required conditions to merge
|
Refacto of the config parsing.
Removed the
RawConfigdeserialization step which was then "parsed" into the finalConfigstruct.Now, the
Configis directly deserialized and validated usingserde_withannotation.It avoids having to update multiple files when we want to add a new field in the config.