-
Notifications
You must be signed in to change notification settings - Fork 31
fix: route RequestBodyPlainText to request_body_data (Devin AI Fix Spike) (do not merge) #862
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…st_body_json This fixes a bug where RequestBodyPlainText was incorrectly routed to request_body_json, causing ValueError: 'Request body json cannot be a string' when using Plain Text body in the Connector Builder. The fix routes RequestBodyPlainText to request_body_data (like RequestBodyUrlEncodedForm) instead of request_body_json. Fixes: airbytehq/oncall#10360 Related: airbytehq/airbyte#69723 Co-Authored-By: unknown <>
Original prompt from API User |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@devin/1764867469-fix-plain-text-body#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch devin/1764867469-fix-plain-text-bodyHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
PyTest Results (Fast)3 818 tests ±0 3 806 ✅ ±0 6m 19s ⏱️ +4s Results for commit acd573a. ± Comparison against base commit daf7d48. This pull request removes 4 and adds 4 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
The test case 'test_string' was testing the old buggy behavior where RequestBodyPlainText was routed to request_body_json. This test has been removed and replaced with a new test case 'test_plain_text_body' in the request_body_data test function to verify the correct behavior. RequestBodyPlainText should route to request_body_data (like RequestBodyUrlEncodedForm) so that plain text is sent as-is without JSON parsing. Co-Authored-By: unknown <>
PyTest Results (Full)3 821 tests ±0 3 809 ✅ ±0 10m 54s ⏱️ -1s Results for commit acd573a. ± Comparison against base commit daf7d48. This pull request removes 4 and adds 4 tests. Note that renamed tests count towards both. |
Summary
Fixes a bug where
RequestBodyPlainTextwas incorrectly routed torequest_body_json, causingValueError: Request body json cannot be a stringwhen using Plain Text body in the Connector Builder.The fix routes
RequestBodyPlainTexttorequest_body_data(likeRequestBodyUrlEncodedForm) instead ofrequest_body_json. This is correct because plain text bodies should be sent via thedata=parameter in requests, notjson=.Root cause: In PR #521,
RequestBodyPlainTextwas incorrectly grouped withRequestBodyJsonObjectto route torequest_body_json. Since plain text is a string (not a dict), this causedhttp_requester.pyline 406 to raiseValueError("Request body json cannot be a string").Fixes: airbytehq/oncall#10360
Related: airbytehq/airbyte#69723
Updates since last revision
test_stringfromtest_interpolated_request_json_using_request_bodythat was testing the old buggy behavior (PlainText → JSON)test_plain_text_bodytotest_interpolated_request_data_using_request_bodyto verify the correct behavior (PlainText → data with interpolation)Review & Testing Checklist for Human
RequestBodyPlainTextshould indeed route torequest_body_data(sent as-is) rather thanrequest_body_json(parsed as JSON). The removed test case assumed PlainText would be JSON-parsed, which conflicts with the bug report.RequestBodyPlainText(POST request with plain text body) and verify it no longer throwsValueError: Request body json cannot be a stringRequestBodyJsonObject,RequestBodyGraphQL, andRequestBodyUrlEncodedFormstill work correctlyRecommended test plan:
Notes
RequestBodyUrlEncodedFormwhich correctly routes torequest_body_datarequest_body_json / request_body_data>request_bodymigration #521), @lmossman (reviewer of fix: (CDK) (Manifest Migration) - fix therequest_body_json / request_body_data>request_bodymigration #521)Devin AI Fix Spike - Created as part of AI triage with confidence score 4/5 (Green)
Link to Devin run: https://app.devin.ai/sessions/fdc2aff723af4077b49e3b9e472e80f2
Requested by: unknown () via
/ai-triageon airbytehq/oncall#10360