fix(clp-rust-utils): Align database name config with updated schema.#1828
Conversation
WalkthroughReplaces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/clp-rust-utils/src/clp_config/package/config.rs (2)
61-67: Add field-level serde default to prevent emptynamesmap.With
#[serde(default)]on the struct, if thenamesfield is omitted during deserialization, serde usesHashMap::default()(empty map), not the populated map fromimpl Default for Database. This leads to the panic risk inmysql.rs.🔎 Proposed fix with field-level default
+fn default_names() -> HashMap<ClpDbNameType, String> { + HashMap::from([ + (ClpDbNameType::Clp, "clp-db".to_owned()), + (ClpDbNameType::Spider, "spider-db".to_owned()), + ]) +} + #[derive(Clone, Debug, Deserialize, Eq, PartialEq)] #[serde(default)] pub struct Database { pub host: String, pub port: u16, + #[serde(default = "default_names")] pub names: HashMap<ClpDbNameType, String>, }
309-311: Consider adding unit tests forDatabaseandClpDbNameTypedeserialization.Given this is a breaking change to the
Databasestruct and introduces a new enum, tests would help verify:
- Correct deserialization when
namesis present with kebab-case keys- Correct default behaviour when
namesis omitted (once the field-level default is added)🔎 Example test
#[test] fn deserialize_database_config() { let json = serde_json::json!({ "host": "db.example.com", "port": 3307, "names": { "clp": "my-clp-db", "spider": "my-spider-db" } }); let db: super::Database = serde_json::from_value(json).expect("failed to deserialize"); assert_eq!(db.host, "db.example.com"); assert_eq!(db.port, 3307); assert_eq!(db.names.get(&super::ClpDbNameType::Clp), Some(&"my-clp-db".to_owned())); assert_eq!(db.names.get(&super::ClpDbNameType::Spider), Some(&"my-spider-db".to_owned())); } #[test] fn deserialize_database_config_with_defaults() { let json = serde_json::json!({ "host": "db.example.com" }); let db: super::Database = serde_json::from_value(json).expect("failed to deserialize"); assert_eq!(db.names.get(&super::ClpDbNameType::Clp), Some(&"clp-db".to_owned())); assert_eq!(db.names.get(&super::ClpDbNameType::Spider), Some(&"spider-db".to_owned())); }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/clp-rust-utils/src/clp_config/package/config.rs(3 hunks)components/clp-rust-utils/src/database/mysql.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
Learnt from: hoophalab
Repo: y-scope/clp PR: 1535
File: components/clp-rust-utils/src/clp_config/package/config.rs:47-61
Timestamp: 2025-11-03T16:17:40.223Z
Learning: In the y-scope/clp repository, the `ApiServer` struct in `components/clp-rust-utils/src/clp_config/package/config.rs` is a Rust-native configuration type and does not mirror any Python code, unlike other structs in the same file (Config, Database, ResultsCache, Package) which are mirrors of Python definitions.
📚 Learning: 2025-11-03T16:17:40.223Z
Learnt from: hoophalab
Repo: y-scope/clp PR: 1535
File: components/clp-rust-utils/src/clp_config/package/config.rs:47-61
Timestamp: 2025-11-03T16:17:40.223Z
Learning: In the y-scope/clp repository, the `ApiServer` struct in `components/clp-rust-utils/src/clp_config/package/config.rs` is a Rust-native configuration type and does not mirror any Python code, unlike other structs in the same file (Config, Database, ResultsCache, Package) which are mirrors of Python definitions.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rscomponents/clp-rust-utils/src/database/mysql.rs
📚 Learning: 2025-11-28T15:12:53.530Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 1655
File: components/clp-package-utils/clp_package_utils/controller.py:183-189
Timestamp: 2025-11-28T15:12:53.530Z
Learning: In the y-scope/clp project, root database credentials (root username and password) are required configuration items for all deployments. The `credentials[ClpDbUserType.ROOT]` entry is guaranteed to exist and can be accessed directly without optional handling.
Applied to files:
components/clp-rust-utils/src/database/mysql.rs
🧬 Code graph analysis (2)
components/clp-rust-utils/src/clp_config/package/config.rs (1)
components/clp-py-utils/clp_py_utils/clp_config.py (2)
ClpDbNameType(211-215)Database(240-405)
components/clp-rust-utils/src/database/mysql.rs (1)
components/clp-py-utils/clp_py_utils/clp_config.py (2)
Database(240-405)ClpDbNameType(211-215)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: check-generated
- GitHub Check: rust-checks
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (2)
components/clp-rust-utils/src/database/mysql.rs (1)
3-6: LGTM on the import update.The import correctly brings in
ClpDbNameTypeneeded for the HashMap key access.components/clp-rust-utils/src/clp_config/package/config.rs (1)
46-52: Enum correctly mirrors PythonClpDbNameType.The
kebab-caseserde renaming matches the PythonKebabCaseStrEnumbehaviour, ensuringClpserializes as"clp"andSpideras"spider"for cross-language compatibility.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/clp-rust-utils/src/clp_config/package/config.rs(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
📚 Learning: 2025-11-03T16:17:40.223Z
Learnt from: hoophalab
Repo: y-scope/clp PR: 1535
File: components/clp-rust-utils/src/clp_config/package/config.rs:47-61
Timestamp: 2025-11-03T16:17:40.223Z
Learning: In the y-scope/clp repository, the `ApiServer` struct in `components/clp-rust-utils/src/clp_config/package/config.rs` is a Rust-native configuration type and does not mirror any Python code, unlike other structs in the same file (Config, Database, ResultsCache, Package) which are mirrors of Python definitions.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
- GitHub Check: check-generated
- GitHub Check: rust-checks
🔇 Additional comments (3)
components/clp-rust-utils/src/clp_config/package/config.rs (3)
1-1: LGTM: Required import for the newnamesfield.The
HashMapimport is necessary for thenamesfield in theDatabasestruct.
46-52: Document the Python source and verify enum matches.The comment referencing
ClpDbNameTypeshould include the full Python module path (e.g.,clp_py_utils.clp_config.ClpDbNameType) for consistency with other mirrored types. Before merging, verify that theClpandSpidervariants and thekebab-caseserde rename match the Python source definition.
66-66: Breaking change properly implemented with HashMap field and key-based access.The transition from
name: Stringtonames: HashMap<ClpDbNameType, String>has been correctly handled. TheDatabasestruct includes appropriate default values for both database types, and usages throughout the codebase (such as inmysql.rs) properly access the map usingconfig.names[&ClpDbNameType::Clp].
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/clp-rust-utils/src/database/mysql.rs (1)
28-28: Direct HashMap indexing remains unsafe despite field-level default.While
config.rsadds#[serde(default = "default_db_names")]to thenamesfield, this only applies when the field is completely missing from the config. If a user providesnames: {}or a partial map (e.g.,names: {spider: "db"}), serde will deserialize the provided value without calling the default function, and the direct indexing here will panic.Replace the direct indexing with safe access and error handling:
🔎 Proposed fix using .get() with error handling
- .database(&config.names[&ClpDbNameType::Clp]) + .database( + config + .names + .get(&ClpDbNameType::Clp) + .ok_or_else(|| { + crate::Error::from("Missing 'clp' database name in configuration") + })?, + )Alternatively, the validation approach suggested in
config.rs(lines 66-67) would catch this at config load time, which is preferable to failing during connection setup.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
components/clp-rust-utils/src/clp_config/package/config.rs(3 hunks)components/clp-rust-utils/src/database/mysql.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
Learnt from: hoophalab
Repo: y-scope/clp PR: 1535
File: components/clp-rust-utils/src/clp_config/package/config.rs:47-61
Timestamp: 2025-11-03T16:17:40.223Z
Learning: In the y-scope/clp repository, the `ApiServer` struct in `components/clp-rust-utils/src/clp_config/package/config.rs` is a Rust-native configuration type and does not mirror any Python code, unlike other structs in the same file (Config, Database, ResultsCache, Package) which are mirrors of Python definitions.
📚 Learning: 2025-11-03T16:17:40.223Z
Learnt from: hoophalab
Repo: y-scope/clp PR: 1535
File: components/clp-rust-utils/src/clp_config/package/config.rs:47-61
Timestamp: 2025-11-03T16:17:40.223Z
Learning: In the y-scope/clp repository, the `ApiServer` struct in `components/clp-rust-utils/src/clp_config/package/config.rs` is a Rust-native configuration type and does not mirror any Python code, unlike other structs in the same file (Config, Database, ResultsCache, Package) which are mirrors of Python definitions.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rscomponents/clp-rust-utils/src/database/mysql.rs
📚 Learning: 2025-10-22T21:02:31.113Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
📚 Learning: 2025-10-22T21:01:31.391Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:01:31.391Z
Learning: In y-scope/clp, for the clp-rust-checks workflow (GitHub Actions) and the Taskfile target deps:lock:check-rust, we should verify Rust Cargo.lock is in sync with Cargo.toml using a non-mutating method (e.g., cargo metadata --locked / cargo check --locked) to keep CI deterministic and avoid updating dependencies during validation.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
📚 Learning: 2025-01-23T17:08:55.566Z
Learnt from: gibber9809
Repo: y-scope/clp PR: 672
File: components/core/src/clp_s/indexer/MySQLIndexStorage.cpp:30-38
Timestamp: 2025-01-23T17:08:55.566Z
Learning: SQL identifiers (table names, column names) in the codebase are currently from trusted sources and directly interpolated into queries. However, the team prefers to implement sanitization for robustness, even for trusted inputs.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
🧬 Code graph analysis (1)
components/clp-rust-utils/src/database/mysql.rs (1)
components/clp-py-utils/clp_py_utils/clp_config.py (2)
ClpDbNameType(211-215)Database(240-405)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: check-generated
- GitHub Check: rust-checks
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
🔇 Additional comments (4)
components/clp-rust-utils/src/clp_config/package/config.rs (3)
1-2: LGTM!The HashMap import is necessary for the new
namesfield and is properly placed.
46-52: LGTM!The
ClpDbNameTypeenum correctly mirrors the Python implementation with:
- Appropriate variants (Clp, Spider)
- Kebab-case serde naming for JSON/YAML serialization
- Hash trait for HashMap key usage
70-78: LGTM!The
Defaultimplementation correctly usesdefault_db_names()to initialize thenamesfield, ensuring consistency with the helper function.components/clp-rust-utils/src/database/mysql.rs (1)
4-4: LGTM!The import of
ClpDbNameTypeis necessary for accessing the HashMap with the enum key.
| #[serde(default = "default_db_names")] | ||
| pub names: HashMap<ClpDbNameType, String>, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find and examine the config.rs file
find . -name "config.rs" -path "*clp_config/package*" -type fRepository: y-scope/clp
Length of output: 115
🏁 Script executed:
#!/bin/bash
# Search for the config.rs file in the clp_config package
fd -t f "config.rs" | grep -E "clp_config|package"
# Also search for mysql.rs
fd -t f "mysql.rs"Repository: y-scope/clp
Length of output: 259
🏁 Script executed:
#!/bin/bash
# Search for ClpDbNameType enum definition
rg -n "enum ClpDbNameType" --type rust
# Search for default_db_names function
rg -n "fn default_db_names" --type rust
# Search for direct indexing on names field
rg -n 'names\s*\[' --type rust -A2 -B2
# Search for mysql.rs line 28
find . -name "mysql.rs" -type f -exec sed -n '20,35p' {} \;Repository: y-scope/clp
Length of output: 1298
🏁 Script executed:
#!/bin/bash
# First, locate the relevant files
echo "=== Finding config.rs files ==="
find . -name "config.rs" | head -20
echo -e "\n=== Finding mysql.rs files ==="
find . -name "mysql.rs" | head -20
echo -e "\n=== Searching for ClpDbNameType enum ==="
rg "enum ClpDbNameType" -A 10 --type rustRepository: y-scope/clp
Length of output: 1147
🏁 Script executed:
#!/bin/bash
# Read the config.rs file around the relevant lines
sed -n '45,95p' components/clp-rust-utils/src/clp_config/package/config.rs
# Search for all direct indexing on names field
echo -e "\n=== Searching for direct indexing on names field ==="
rg 'names\s*\[\s*&ClpDbNameType' --type rust -B2 -A2
# Search for Spider key usage
echo -e "\n=== Searching for Spider key usage ==="
rg 'ClpDbNameType::Spider' --type rust -B2 -A2
# Check for .get() patterns on names
echo -e "\n=== Checking for safe .get() patterns ==="
rg 'names\.get\(' --type rust -B2 -A2Repository: y-scope/clp
Length of output: 2391
🏁 Script executed:
#!/bin/bash
# Read the default_db_names function
sed -n '75,95p' components/clp-rust-utils/src/clp_config/package/config.rs
# Check for any other indexing patterns
rg 'config\.names\[' --type rust -B2 -A2Repository: y-scope/clp
Length of output: 982
Ajoutez une validation post-désérialisation pour s'assurer que les clés requises sont présentes.
L'attribut #[serde(default = "default_db_names")] au niveau du champ ne s'applique que lorsque le champ names est complètement absent de la configuration. Si un utilisateur fournit une configuration partielle comme names: {} ou names: {spider: "db"}, la fonction par défaut ne sera pas appelée, ce qui laisse le HashMap sans la clé Clp. Cela provoquera une panique à la ligne 28 de mysql.rs où l'indexation directe est utilisée.
Envisagez d'ajouter une validation post-désérialisation pour garantir que toutes les clés requises (Clp et Spider) sont présentes :
impl Database {
pub fn validate(&self) -> Result<(), String> {
if !self.names.contains_key(&ClpDbNameType::Clp) {
return Err("Missing required 'clp' database name in configuration".to_string());
}
if !self.names.contains_key(&ClpDbNameType::Spider) {
return Err("Missing required 'spider' database name in configuration".to_string());
}
Ok(())
}
}Appelez cette validation après la désérialisation de la configuration et avant de l'utiliser.
🤖 Prompt for AI Agents
In components/clp-rust-utils/src/clp_config/package/config.rs around lines
66-67, the serde default for `names` only applies if the whole field is missing,
so partially provided maps can lack required keys and cause a panic; add a
post-deserialization validation method on the Database struct that checks
presence of both Clp and Spider keys in the names HashMap and returns a clear
Err when missing, and ensure this validate() is invoked immediately after
deserializing the config (before any usage) so callers fail fast with a
descriptive error instead of panicking.
There was a problem hiding this comment.
pouvez-vous parler anglais? je ne comprends pas
There was a problem hiding this comment.
@junhaoliao, I apologize for responding in French! Let me provide my feedback in English.
Add post-deserialization validation to ensure required keys are present.
The #[serde(default = "default_db_names")] attribute on the field only applies when the names field is completely absent from the configuration. If a user provides a partial configuration like names: {} or names: {spider: "db"}, the default function won't be called, leaving the HashMap without the Clp key. This will cause a panic at line 28 in mysql.rs where direct indexing is used.
Consider adding post-deserialization validation to ensure all required keys (Clp and Spider) are present:
impl Database {
pub fn validate(&self) -> Result<(), String> {
if !self.names.contains_key(&ClpDbNameType::Clp) {
return Err("Missing required 'clp' database name in configuration".to_string());
}
if !self.names.contains_key(&ClpDbNameType::Spider) {
return Err("Missing required 'spider' database name in configuration".to_string());
}
Ok(())
}
}Call this validation after deserializing the configuration and before using it.
🧠 Learnings used
Learnt from: hoophalab
Repo: y-scope/clp PR: 1535
File: components/clp-rust-utils/src/clp_config/package/config.rs:47-61
Timestamp: 2025-11-03T16:17:40.223Z
Learning: In the y-scope/clp repository, the `ApiServer` struct in `components/clp-rust-utils/src/clp_config/package/config.rs` is a Rust-native configuration type and does not mirror any Python code, unlike other structs in the same file (Config, Database, ResultsCache, Package) which are mirrors of Python definitions.
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @components/clp-rust-utils/src/clp_config/package/config.rs:
- Around line 44-66: ClpDbNames currently allows empty strings after
deserialization which violates the Python NonEmptyStr constraint; add a
validation method on ClpDbNames (e.g., pub fn validate(&self) -> Result<(),
String>) that returns Err if clp or spider is empty, and call this validate()
immediately after deserializing your configuration (before any DB
construction/usage) so errors are surfaced early; reference the ClpDbNames
struct and ensure callers that parse config propagate or handle the validation
error.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
components/clp-rust-utils/src/clp_config/package/config.rs
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
📚 Learning: 2025-11-03T16:17:40.223Z
Learnt from: hoophalab
Repo: y-scope/clp PR: 1535
File: components/clp-rust-utils/src/clp_config/package/config.rs:47-61
Timestamp: 2025-11-03T16:17:40.223Z
Learning: In the y-scope/clp repository, the `ApiServer` struct in `components/clp-rust-utils/src/clp_config/package/config.rs` is a Rust-native configuration type and does not mirror any Python code, unlike other structs in the same file (Config, Database, ResultsCache, Package) which are mirrors of Python definitions.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
📚 Learning: 2025-10-22T21:02:31.113Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:02:31.113Z
Learning: Repository y-scope/clp: Maintain deterministic CI/builds for Rust; add a check to verify Cargo.lock is in sync with Cargo.toml without updating dependencies (non-mutating verification in clp-rust-checks workflow).
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
📚 Learning: 2025-10-22T21:01:31.391Z
Learnt from: junhaoliao
Repo: y-scope/clp PR: 0
File: :0-0
Timestamp: 2025-10-22T21:01:31.391Z
Learning: In y-scope/clp, for the clp-rust-checks workflow (GitHub Actions) and the Taskfile target deps:lock:check-rust, we should verify Rust Cargo.lock is in sync with Cargo.toml using a non-mutating method (e.g., cargo metadata --locked / cargo check --locked) to keep CI deterministic and avoid updating dependencies during validation.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
📚 Learning: 2025-01-23T17:08:55.566Z
Learnt from: gibber9809
Repo: y-scope/clp PR: 672
File: components/core/src/clp_s/indexer/MySQLIndexStorage.cpp:30-38
Timestamp: 2025-01-23T17:08:55.566Z
Learning: SQL identifiers (table names, column names) in the codebase are currently from trusted sources and directly interpolated into queries. However, the team prefers to implement sanitization for robustness, even for trusted inputs.
Applied to files:
components/clp-rust-utils/src/clp_config/package/config.rs
🧬 Code graph analysis (1)
components/clp-rust-utils/src/clp_config/package/config.rs (1)
components/clp-py-utils/clp_py_utils/clp_config.py (1)
Database(240-405)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: package-image
- GitHub Check: lint-check (ubuntu-24.04)
- GitHub Check: lint-check (macos-15)
- GitHub Check: check-generated
- GitHub Check: rust-checks
🔇 Additional comments (1)
components/clp-rust-utils/src/clp_config/package/config.rs (1)
80-80: LGTM! Database struct correctly updated.The replacement of
name: Stringwithnames: ClpDbNamescorrectly aligns with the Pythonclp-py-utilsDatabase model. The updated default implementation properly initializes the new structure.Also applies to: 88-88
clp-rust-utils Database config with names field to match clp-py-utils Database model.There was a problem hiding this comment.
validated by:
- changing the default clp db name in
build/clp-package/etc/clp-config.yaml:database: type: "mariadb" # "mariadb" or "mysql" host: "localhost" port: 3306 names: clp: "my-clp-db" spider: "my-spider-db"
cd build/clp-package; ./sbin/start-clp.sh- Observed the package was started successfully, including the api server service.
-
curl -s -X POST http://localhost:3001/query \ -H "Content-Type: application/json" \ -d '{ "query_string": "*", "dataset": "default", "ignore_case": false, "max_num_results": 10, "write_to_file": false }' curl -s -N http://localhost:3001/query_results/1 | head -3and observed results returned-
docker compose -p clp-package-500a logs api-server api-server-1 | {"timestamp":"2026-01-07T14:33:46.722508Z","level":"INFO","fields":{"message":"Server started at 0.0.0.0:3001"},"filename":"components/api-server/src/bin/api_server.rs","line_number":101} api-server-1 | {"timestamp":"2026-01-07T14:37:57.195474Z","level":"INFO","fields":{"message":"Submitting query: QueryConfig { query_string: \"*\", dataset: Some(\"default\"), max_num_results: 10, begin_timestamp: None, end_timestamp: None, ignore_case: false, write_to_file: false }"},"filename":"components/api-server/src/routes.rs","line_number":108} api-server-1 | {"timestamp":"2026-01-07T14:37:57.209891Z","level":"INFO","fields":{"message":"Submitted query with search job ID: 1"},"filename":"components/api-server/src/routes.rs","line_number":111} api-server-1 | {"timestamp":"2026-01-07T14:38:13.884597Z","level":"INFO","fields":{"message":"Fetching results for search job ID: 1"},"filename":"components/api-server/src/routes.rs","line_number":153} api-server-1 | {"timestamp":"2026-01-07T14:38:13.886642Z","level":"INFO","fields":{"message":"Successfully initiated result stream for search job ID 1"},"filename":"components/api-server/src/routes.rs","line_number":156}
for the title, how about:
fix(clp-rust-utils): Align database name config with updated schema.
|
sorry i think we should update the Helm chart too; otherwise the api server deployments would fail to start: @coderabbitai create an issue to track |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1355 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 41 🏁 Script executed: Length of output: 30 🏁 Script executed: Length of output: 4617 🏁 Script executed: Length of output: 8364 🏁 Script executed: Length of output: 42 @junhaoliao I've created issue #1845 to track the Helm chart update: #1845 The issue documents the need to add the 🧠 Learnings used |
…-scope#1828) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> Co-authored-by: LinZhihao-723 <zh.lin@mail.utoronto.ca>
…-scope#1828) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> Co-authored-by: LinZhihao-723 <zh.lin@mail.utoronto.ca>
…-scope#1828) Co-authored-by: Junhao Liao <junhao.liao@yscope.com> Co-authored-by: LinZhihao-723 <zh.lin@mail.utoronto.ca>
Description
This PR updates the
Databaseconfig struct inclp-rust-utilsto match the updatednamesfield in its Python counterpart. TheClpDbNameTypeenum was also added.Checklist
breaking change.
Validation performed
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.