Upgrade API Documentation to be Python 3 Compatible#1304
Upgrade API Documentation to be Python 3 Compatible#1304jess-tech-lab wants to merge 10 commits intoOWASP:masterfrom
Conversation
Signed-off-by: jess-tech-lab <221731682+jess-tech-lab@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Summary by CodeRabbit
WalkthroughAdds a note about disabling SSL warnings for local Nettacker API with self-signed certificates and updates HTTP examples to use 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🧪 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 (2)
docs/API.md (2)
82-82: Preferr.json()overjson.loads(r.content.decode('utf-8'))for JSON responses.The
requestslibrary exposesr.json()which handles decoding and parsing in one step, andr.textfor plain decoded responses. Both are the idiomatic Python 3 alternatives throughout the file:♻️ Suggested simplifications (representative lines; pattern applies to all similar occurrences)
->>> print(json.dumps(json.loads(r.content.decode('utf-8')), sort_keys=True, indent=4)) +>>> print(json.dumps(r.json(), sort_keys=True, indent=4))->>> r.content.decode('utf-8') +>>> r.text->>> print(r.content.decode('utf-8')) +>>> print(r.text)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/API.md` at line 82, Replace direct decoding and parsing of response content with the idiomatic requests helpers: where the code uses patterns like print(json.dumps(json.loads(r.content.decode('utf-8')), sort_keys=True, indent=4)) (variable r), swap to print(json.dumps(r.json(), sort_keys=True, indent=4)) for JSON responses and use r.text for plain decoded text; update all similar occurrences that call json.loads(r.content.decode(...)) to r.json() and any manual .content.decode(...) usage to r.text.
229-244: Sets.verify = Falseonce on the session rather than per-request.Passing
verify=Falseto every individuals.get()call is repetitive. Since the whole session targets the same self-signed endpoint, disabling verification at the session level is cleaner and consistent with howrequests.Sessionis designed to be used.♻️ Proposed simplification
>>> s = requests.session() +>>> s.verify = False ->>> r = s.get("https://localhost:5000/session/set?key=09877e92c75f6afdca6ae61ad3f53727", verify=False) +>>> r = s.get("https://localhost:5000/session/set?key=09877e92c75f6afdca6ae61ad3f53727")Apply the same removal of
verify=Falseto all subsequents.get()calls in the session sections.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/API.md` around lines 229 - 244, The examples repeatedly pass verify=False to each s.get call; instead, set the session-level flag once by assigning s.verify = False immediately after creating the session (after s = requests.session()) and remove all per-request verify=False arguments from subsequent calls to s.get (and any other session requests) so the session consistently uses the disabled verification for the self-signed endpoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/API.md`:
- Around line 59-68: Add a brief security note under the "## Requests Structure"
heading explaining that the examples call
disable_warnings(InsecureRequestWarning) and use verify=False which disables TLS
certificate validation, clarifying that this is only for the local Nettacker API
with a self-signed cert and must not be used against external/production hosts;
mention both disable_warnings(InsecureRequestWarning) and verify=False by name
so readers understand which patterns are unsafe for production.
---
Nitpick comments:
In `@docs/API.md`:
- Line 82: Replace direct decoding and parsing of response content with the
idiomatic requests helpers: where the code uses patterns like
print(json.dumps(json.loads(r.content.decode('utf-8')), sort_keys=True,
indent=4)) (variable r), swap to print(json.dumps(r.json(), sort_keys=True,
indent=4)) for JSON responses and use r.text for plain decoded text; update all
similar occurrences that call json.loads(r.content.decode(...)) to r.json() and
any manual .content.decode(...) usage to r.text.
- Around line 229-244: The examples repeatedly pass verify=False to each s.get
call; instead, set the session-level flag once by assigning s.verify = False
immediately after creating the session (after s = requests.session()) and remove
all per-request verify=False arguments from subsequent calls to s.get (and any
other session requests) so the session consistently uses the disabled
verification for the self-signed endpoint.
Signed-off-by: jess-tech-lab <221731682+jess-tech-lab@users.noreply.github.com>
Proposed change
This PR updates the API document API.md to transition all Python examples from Python 2 to Python 3.10+. The current documentation contains deprecated syntax including
print ..., theu'string prefix, and missing byte decoding, which leads to errors in Python 3.Proposed changes are as follows:
r.contentexamples to include.decode('utf-8')to prevent TypeError in Python 3.verify=Falseto all example requests to prevent the errorcertificate verify failed: self-signed certificatesince by default Python 3 enforces certificate verification and the API uses a self-signed certificate.print ...toprint(...)and removed Python 2 specific unicode prefixu'.Type of change
Checklist
make pre-commit, it didn't generate any changesmake test, all tests passed locally