add wikidata update capability to the flask api#212
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new POST /update/wikidata endpoint to the Flask app that expects JSON with a Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Flask as FlaskApp
participant Importer as WikidataImporter
participant WDAPI as Wikidata API
Client->>Flask: POST /update/wikidata { "qid": "Q..." }
Flask->>Flask: validate JSON & extract qid
Flask->>Importer: new WikidataImporter()
Flask->>Importer: update_entities(id_list=[qid])
Importer->>WDAPI: fetch/update entity Q...
WDAPI-->>Importer: entity data / status
Importer-->>Flask: update result
Flask-->>Client: 200 { ...result... }
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
flask_app/app.py (1)
212-217: Docstring is inaccurate.The docstring says "Update person profile from wikidata" but the
update_entitiesmethod handles general entities (items and properties), not just person profiles. The return description mentions "an empty list" butupdate_entitiesreturns either a single MaRDI ID string or a dict of updated entities.📝 Suggested docstring update
`@app.post`("/update/wikidata") def update_wikidata(): - """Update person profile from wikidata + """Update a Wikidata entity in the local Wikibase. - Expects JSON with a ``qid`` field, which should contain the QID of a wikidata item. + Expects JSON with a ``qid`` field containing a Wikidata item or property ID. Returns: - Response, either the QID that was updated or an empty list + Flask response tuple with the updated MaRDI ID or import result. """🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flask_app/app.py` around lines 212 - 217, The docstring for update_entities is misleading and should be updated: change the summary from “Update person profile from wikidata” to indicate it updates general Wikidata entities (items and properties), update the input description to reflect the expected JSON payload (e.g., a single "qid" or a collection of QIDs as accepted by update_entities), and correct the return description to state that the function returns either a single MaRDI ID string for single-entity updates or a dict mapping QIDs to update results for batch updates; update_entities is the function to modify.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flask_app/app.py`:
- Around line 210-225: The update_wikidata view lacks logging, JSON-wrapped
responses, and exception handling: add log.info("Called 'update_wikidata'.") at
the start of update_wikidata, call WikidataImporter() and
wdi.update_entities(id_list=[qid]) inside a try block, wrap the successful
response in jsonify(...) before returning, catch LoginError from
WikidataImporter and return an appropriate auth error (e.g., jsonify(error=...)
with a 401/503 code), catch ValueError from update_entities and return
jsonify(error=...) with 400, and catch a generic Exception to log the exception
and return jsonify(error="internal error") with 500; reference the
functions/classes/names WikidataImporter, update_entities, update_wikidata,
request.get_json, and log.info to locate where to apply these changes.
---
Nitpick comments:
In `@flask_app/app.py`:
- Around line 212-217: The docstring for update_entities is misleading and
should be updated: change the summary from “Update person profile from wikidata”
to indicate it updates general Wikidata entities (items and properties), update
the input description to reflect the expected JSON payload (e.g., a single "qid"
or a collection of QIDs as accepted by update_entities), and correct the return
description to state that the function returns either a single MaRDI ID string
for single-entity updates or a dict mapping QIDs to update results for batch
updates; update_entities is the function to modify.
| @app.post("/update/wikidata") | ||
| def update_wikidata(): | ||
| """Update person profile from wikidata | ||
|
|
||
| Expects JSON with a ``qid`` field, which should contain the QID of a wikidata item. | ||
|
|
||
| Returns: | ||
| Response, either the QID that was updated or an empty list | ||
| """ | ||
| data = request.get_json(silent=True) or {} | ||
| qid = data.get("qid") | ||
| if not qid: | ||
| return jsonify(error="missing qid"), 400 | ||
| wdi = WikidataImporter() | ||
| response = wdi.update_entities(id_list=[qid]) | ||
| return response, 200 |
There was a problem hiding this comment.
Missing exception handling will cause unhandled 500 errors.
Based on the relevant code snippets, WikidataImporter() can raise LoginError on authentication failure, and update_entities() can raise ValueError for invalid QID formats (e.g., IDs not starting with 'Q' or 'P'). Other endpoints in this file consistently wrap external calls in try/except blocks.
Additionally:
- The response should be wrapped in
jsonify()for consistency and proper JSON serialization (especially whenupdate_entitiesreturns a string). - Missing
log.info("Called 'update_wikidata'.")at the start, which other endpoints include.
🔧 Proposed fix with exception handling and JSON serialization
`@app.post`("/update/wikidata")
def update_wikidata():
"""Update person profile from wikidata
Expects JSON with a ``qid`` field, which should contain the QID of a wikidata item.
Returns:
Response, either the QID that was updated or an empty list
"""
+ log.info("Called 'update_wikidata'.")
+
data = request.get_json(silent=True) or {}
qid = data.get("qid")
if not qid:
return jsonify(error="missing qid"), 400
- wdi = WikidataImporter()
- response = wdi.update_entities(id_list=[qid])
- return response, 200
+
+ try:
+ wdi = WikidataImporter()
+ response = wdi.update_entities(id_list=[qid])
+ return jsonify(result=response), 200
+ except ValueError as exc:
+ log.error("Invalid QID format: %s", exc)
+ return jsonify(error="invalid qid format", details=str(exc)), 400
+ except Exception as exc:
+ log.error("Failed to update wikidata entity: %s", exc, exc_info=True)
+ return jsonify(error="update failed", details=str(exc)), 500🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flask_app/app.py` around lines 210 - 225, The update_wikidata view lacks
logging, JSON-wrapped responses, and exception handling: add log.info("Called
'update_wikidata'.") at the start of update_wikidata, call WikidataImporter()
and wdi.update_entities(id_list=[qid]) inside a try block, wrap the successful
response in jsonify(...) before returning, catch LoginError from
WikidataImporter and return an appropriate auth error (e.g., jsonify(error=...)
with a 401/503 code), catch ValueError from update_entities and return
jsonify(error=...) with 400, and catch a generic Exception to log the exception
and return jsonify(error="internal error") with 500; reference the
functions/classes/names WikidataImporter, update_entities, update_wikidata,
request.get_json, and log.info to locate where to apply these changes.
add wikidata update capability to flask app
Summary by CodeRabbit
New Features
Known Issues