fix: lazy imports in sql_connection_manager, remove hardcoded $set wrapper - #504
fix: lazy imports in sql_connection_manager, remove hardcoded $set wrapper#504saurabhhhcodes wants to merge 1 commit into
Conversation
… mongo_document_update
|
👋 Thank you for opening this pull request! We're excited to review your contribution. Please give us a moment, and we'll get back to you shortly! Feel free to join our community on Discord to discuss more! |
📝 WalkthroughWalkthroughMongoDB updates now accept complete MongoDB update documents, while MySQL and PostgreSQL drivers load only when their connection functions execute. ChangesMongoDB update document contract
Connection-scoped SQL driver imports
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pysnippets/database/mongodb/mongo_document_update.py`:
- Around line 15-17: Restore automatic $set wrapping for update_data in the
update_many and update_one branches so callers can continue passing plain field
data such as {"age": 31}. Preserve the existing filter_criteria and multi-update
selection behavior; do not switch the API contract to require full MongoDB
update documents.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8da0e08d-0158-41fa-b034-084cb65b4799
📒 Files selected for processing (2)
pysnippets/database/mongodb/mongo_document_update.pypysnippets/database/mysql/sql_connection_manager.py
| result = collection.update_many(filter_criteria, update_data) | ||
| else: | ||
| result = collection.update_one(filter_criteria, {'$set': update_data}) | ||
| result = collection.update_one(filter_criteria, update_data) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files matching target =="
fd -a 'mongo_document_update.py|database.md|test_mongo.py' . || true
echo "== target snippet and surrounding context =="
if [ -f pysnippets/database/mongodb/mongo_document_update.py ]; then
wc -l pysnippets/database/mongodb/mongo_document_update.py
cat -n pysnippets/database/mongodb/mongo_document_update.py
fi
echo "== doc references =="
if [ -f pysnippets/database/database.md ]; then
nl -ba pysnippets/database/database.md | sed -n '1,220p'
fi
echo "== test references =="
if [ -f Tests/database/test_mongo.py ]; then
wc -l Tests/database/test_mongo.py
nl -ba Tests/database/test_mongo.py | sed -n '1,260p'
fi
echo "== search update_many/update_one usages in repo =="
rg -n "update_many|update_one|mongo_document_update|update_data|set.*age|age.*31" . || trueRepository: UTSAVS26/PySnippets
Length of output: 1324
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== database.md =="
if [ -f pysnippets/database/database.md ]; then
awk '{printf "%6d\t%s\n", NR, $0}' pysnippets/database/database.md | sed -n '1,240p'
fi
echo "== test_mongo.py =="
if [ -f Tests/database/test_mongo.py ]; then
awk '{printf "%6d\t%s\n", NR, $0}' Tests/database/test_mongo.py | sed -n '1,280p'
fi
echo "== relevant repository usages =="
rg -n "update_many|update_one|update_document|update_data|set|age.*31|\$set" . || true
echo "== repository commit diff stat/name-only =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git diff --stat 2>/dev/null || true
git diff --name-only HEAD~1..HEAD 2>/dev/html|sed -n '1,200p' || trueRepository: UTSAVS26/PySnippets
Length of output: 50376
🌐 Web query:
PyMongo collection update_one update document operator required behavior passing field update updateData
💡 Result:
In PyMongo, the update_one() method requires an update document that specifies the modifications to apply to the matched document [1][2]. This update document must use MongoDB update operator expressions (e.g., $set, $inc, $unset) to modify specific fields [1][3][4]. Key Requirements and Behavior: 1. Update Operators: The update parameter must be a mapping (dictionary) that contains update operator expressions [1][2]. For example, to update a field named "status" to "active", you must pass {'$set': {'status': 'active'}} rather than simply passing {'status': 'active'} [3]. Passing a naked document (without operators) as the update parameter is generally incorrect for update_one() and will typically cause an error or unexpected behavior, as MongoDB will interpret it as an attempt to replace the entire document content if used in a replacement context, which update_one() does not support [3]. 2. Update Document Structure: - The update document must be a dictionary-like object [2]. - It specifies the change to perform using operators like $set (to modify a field), $inc (to increment a numeric field), or $unset (to remove a field) [4][5]. 3. Alternative (Aggregation Pipeline): Starting in MongoDB 4.2 and supported by PyMongo, the update parameter can also be an aggregation pipeline (a list of stages) instead of an update operator document [6][3][2]. This allows for more complex updates, such as setting a field based on the value of another field [3]. Example of standard usage: collection.update_one({'_id': 1}, {'$set': {'field_to_update': 'new_value'}}) Summary: Always wrap your field updates in appropriate operators like $set within the update dictionary to ensure correct modification of the targeted document [1][3].
Citations:
- 1: https://www.mongodb.com/docs/languages/python/pymongo-driver/current/crud/update/
- 2: https://pymongo.readthedocs.io/en/stable/api/pymongo/collection.html
- 3: https://www.mongodb.com/docs/manual/reference/method/db.collection.updateone/
- 4: https://www.mongodb.com/docs/manual/reference/operator/update-field/
- 5: https://www.mongodb.com/docs/manual/reference/mql/update/
- 6: https://pymongo.readthedocs.io/en/stable/api/pymongo/operations.html
Preserve the automatic $set wrapping or update the API contract.
update_data is still documented as plain field data ({"age": 31}), and Tests/database/test_mongo.py expects $set wrapping. Passing those values directly to PyMongo makes this a document replacement-style update contract rather than a targeted field update. Either wrap update_data with $set again when callers provide field data, or change all caller/test expectations to full MongoDB update documents.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pysnippets/database/mongodb/mongo_document_update.py` around lines 15 - 17,
Restore automatic $set wrapping for update_data in the update_many and
update_one branches so callers can continue passing plain field data such as
{"age": 31}. Preserve the existing filter_criteria and multi-update selection
behavior; do not switch the API contract to require full MongoDB update
documents.
1 participant
Bug 1: Module-level imports causing ImportError
sql_connection_manager.pyimportedpsycopg2andmysql.connectorat module level. Installing either module alone would cause an ImportError for the other, preventing use of the module entirely.Fix: Moved imports inside their respective functions (lazy imports).
Bug 2: Hardcoded $set wrapper in mongo_document_update
update_documentalways wrappedupdate_datain{"$set": ...}, making it impossible to use MongoDB operators like$inc,$unset,$push,$pull, etc.Fix: Pass
update_datadirectly toupdate_one/update_manywithout wrapping.Summary by CodeRabbit
$set.