Added Configuration Management Operations

[MinuraPunchihewa]

This PR adds configuration management to the server object allowing components like the default models to be set via the SDK.

[entelligence-ai-pr-reviews]

Review Summary

🏷️ Draft Comments (5)

Skipped posting 5 drafted comments based on your review threshold. Feel free to update them here.

mindsdb_sdk/config.py (3)

67-73: api_key is included in the config dict even when api_key=None, which may overwrite or leak sensitive config values with nulls.

Scores:

• Production Impact: 3
• Fix Specificity: 5
• Urgency Impact: 3
• Total Score: 11

Reason for filtering: Valid issue - the code at lines 67-73 always includes api_key in the config dict even when None, which could overwrite existing config values or store null values unnecessarily. The commitable suggestion provides a clean conditional inclusion pattern that would work correctly.

Analysis: This is a legitimate configuration handling issue. Including None values in config dictionaries can cause problems when merging configurations or when the backend expects certain keys to be absent rather than null. The fix is specific and immediately applicable.

---

101-106: api_key is included in the embedding model config even when api_key=None, which may overwrite or leak sensitive config values with nulls.

Scores:

• Production Impact: 3
• Fix Specificity: 5
• Urgency Impact: 3
• Total Score: 11

Reason for filtering: Valid issue - identical problem to the first comment but in the embedding model configuration at lines 101-106. The api_key is always included even when None, and the suggested fix is appropriate.

Analysis: Same configuration handling issue as the LLM config. The pattern of always including api_key regardless of its None value is problematic for config management. The conditional inclusion fix is clean and correct.

---

134-139: api_key is included in the reranking model config even when api_key=None, which may overwrite or leak sensitive config values with nulls.

Scores:

• Production Impact: 3
• Fix Specificity: 5
• Urgency Impact: 3
• Total Score: 11

Reason for filtering: Valid issue - same problem exists in the reranking model configuration at lines 134-139. The api_key handling is inconsistent with best practices for optional configuration values.

Analysis: Third instance of the same configuration pattern issue. All three methods have identical problems with api_key handling. The suggested fix maintains consistency across all configuration methods and prevents null value pollution in configs.

---

mindsdb_sdk/connectors/rest_api.py (2)

222-231: get_file_metadata fetches all file metadata and iterates to find one file, causing O(n) time and unnecessary data transfer for large file sets.

Scores:

• Production Impact: 3
• Fix Specificity: 2
• Urgency Impact: 2
• Total Score: 7

Reason for filtering: Valid performance concern - the method fetches all file metadata and iterates through it to find one file, which is inefficient for large datasets. The line numbers correctly identify the problematic code at lines 221-231.

Analysis: This is a legitimate O(n) performance issue that could impact production with large file sets. The comment correctly identifies the inefficient pattern and provides context about the missing endpoint. While not immediately breaking, it's a scalability concern that should be addressed.

---

483-491: update_config allows arbitrary configuration updates via user-supplied dict without authentication or authorization checks, enabling privilege escalation or denial-of-service if exposed to untrusted users.

Scores:

• Production Impact: 1
• Fix Specificity: 1
• Urgency Impact: 1
• Total Score: 3

Reason for filtering: The commitable suggestion is flawed - it assumes authentication state can be determined by checking local credentials, but the session may already be authenticated via cookies or other means. The authentication check logic is incorrect and could break legitimate usage. Additionally, this is likely a client-side SDK where server-side authorization should handle access control.

Analysis: While the security concern about unrestricted config updates has merit, the suggested fix is technically incorrect. The authentication check doesn't account for session-based auth or cookies, and client-side validation is inappropriate for authorization. The server should handle access control, not the SDK client.

---

🔍 Comments beyond diff scope (1)

mindsdb_sdk/server.py (1)

33-33: super().__init__(self, api, 'mindsdb') passes self as the first argument, which is incorrect and can cause runtime errors or unexpected behavior in the class hierarchy.
Category: correctness

---

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
mindsdb_sdk
agents.py	211	56	73%	33, 101, 104, 107, 110, 118, 126, 146, 167, 178, 181, 185, 187, 189, 191, 193, 195, 257, 270, 281, 292, 296–311, 323, 332–336, 343–344, 392, 400–401, 454, 503–504, 507, 514, 535–537, 541–545
databases.py	49	2	96%	118, 146
handlers.py	39	1	97%	77
jobs.py	97	7	93%	40, 52, 80, 84, 146–149
knowledge_bases.py	133	15	89%	70–73, 134, 160, 189, 196, 200–202, 206, 228–232, 242
ml_engines.py	42	3	93%	94, 126, 128
models.py	210	19	91%	109, 140–141, 222, 231, 233, 303, 339, 348, 372, 397, 491, 499, 518, 534, 542, 567, 571, 584
projects.py	63	1	98%	160
query.py	13	1	92%	14
skills.py	53	3	94%	43, 45, 49
tables.py	130	15	88%	140–142, 145, 165, 192, 203–204, 209, 224, 227, 321, 342–347, 356, 376
views.py	37	2	95%	105, 138
mindsdb_sdk/connectors
rest_api.py	264	52	80%	19–29, 35–36, 51, 55, 58–59, 79–81, 102, 105, 112–115, 148–156, 177–178, 213–216, 230–231, 285–290, 294–306
mindsdb_sdk/utils
agents.py	50	4	92%	72, 79–81
mind.py	47	47	0%	1–128
openai.py	85	30	65%	37–40, 83–85, 107, 148–158, 215–216, 234–240, 258–276
table_schema.py	21	21	0%	1–54
TOTAL	1652	279	83%

Tests	Skipped	Failures	Errors	Time
31	0 💤	0 ❌	0 🔥	10.897s ⏱️