feat: add custom JSON serializer support to HTTPClient#1590
feat: add custom JSON serializer support to HTTPClient#1590Gardner-Programs wants to merge 2 commits into
Conversation
Allow configuring a custom JSON serializer via Client.set_serializer / HTTPClient.set_serializer, applied to all request bodies. Useful for encoding types the stdlib json module can't handle by default (e.g. datetime, Decimal). Defaults to None, preserving existing behavior. Closes burnash#1556
| headers: Optional[MutableMapping[str, str]] = None, | ||
| ) -> Response: | ||
| if self.serializer is not None and json is not None: | ||
| data = self.serializer(dict(json)) |
There was a problem hiding this comment.
I was just mirroring the existing json=dict(json) if json else None line below it. Since json is typed as Mapping[str, Any] it isn't guaranteed to be a concrete dict, so this makes sure the serializer gets one it can encode.
|
cool, that would definitely solve the problem I had and I think your point about putting this at the higher level makes total sense 👍 |
|
Hey @Gardner-Programs & @tlienart, Can't we implement something simpler to fix the issue? I'm not a fan of ...
... because it's a global state. Wouldn't it make more sense to let users serialize their data before running the update call? We'd have to implement the following:
User would be able to run ws.update(
[[datetime.date(2026, 6, 23)]],
"A1",
serializer=lambda b: json.dumps(b, default=lambda o: o.isoformat()),
)Overall, we'd achieve the same but we'd remove layers of complexity. What do you think? |
|
Hey @antoineeripret, thanks for taking a look 🙏 I actually modeled On why I went this way: in my own usage my serialization needs are per-spreadsheet, not per-call. A given script talks to one spreadsheet and every write needs the same handling (e.g. The other thing that pushed me toward the setter is coverage. It hooks in at That said, here's a possible compromise. What if we support both instead of picking one? Keep ws.update(
[[datetime.date(2026, 6, 23)]],
"A1",
serializer=lambda b: json.dumps(b, default=lambda o: o.isoformat()),
)while folks with uniform per-spreadsheet needs can still set it once. It stays fairly small because I do want to be honest about the trade-offs though:
Happy to go whichever direction you think is best, whether that's the setter as-is, the per-call param on its own, or the combined approach. Let me know what you'd prefer and I'll adjust. |
|
That's a very good point you raise, I actually have other scripts with other libraries where I have to do just that, and it's a pain 😢. That being said, I wouldn't go with the double-implementation because I'm unsure it's a good idea for a library that is currently short of maintainers. Let me suggest a different approach as a compromise, w/o adding too much complexity.
import json, functools
import gspread
def _iso_serializer(body):
return json.dumps(body, default=lambda o: o.isoformat())
#we define the per_spreadsheet here
def open_sheet(name):
gc = gspread.oauth()
ws = gc.open(name).sheet1
ws.update = functools.partial(ws.update, serializer=_iso_serializer)
ws.batch_update = functools.partial(ws.batch_update, serializer=_iso_serializer)
return ws
ws = open_sheet("Report A")
ws.update(...) # serializer already baked inWhat do you think? Please do not hesitate to let me know if you think that your approach is better: I don't want to push mine, I just want to make sure we implement the right solution given our objectives and the library's context. Thank you ! |
|
Hey @antoineeripret, the setter is what worked for my personal usage, but I agree it's not the best call for the repo. Let's go per-call and drop it. The global default fit the majority of the cases I've personally hit, so it felt like the natural choice at the time. But I've never worked with a team that shared a single client across multiple scripts or functions, and once you do, I can see how a mutable setter would cause real problems. That's a context I just wasn't designing for. The One small thing I wanted to run by you before I rework it. Instead of a full serializer that the user owns: ws.update([[date(2026, 6, 23)]], "A1",
serializer=lambda b: json.dumps(b, default=lambda o: o.isoformat()))what if we expose just the ws.update([[date(2026, 6, 23)]], "A1", json_default=str) # str covers datetime, Decimal, UUID...Same result for the datetime/Decimal case, but since the library still owns I'm happy either way, so just let me know which you'd prefer. One thing I want to flag with the per-call approach: unlike the setter, it only applies to the methods we thread it through, so any future data-writing method would need the same param to support it. Nothing blocking, just worth deciding up front. Do you want me to thread it through |
|
Hey @Gardner-Programs, Great idea, really love it ! I'd maybe use a different name for the parameter though: the name makes sense if you already know the Regarding your questions: 1/ Which methods to thread it through: I'd add Thank you ! |
|
Perfect, let's lock it in. On the name, I went with For scope I'll thread it through I'll get the rework up shortly. Thanks for the back and forth on this! |
Replace the client-wide set_serializer() with a per-call default_serializer argument on update, batch_update, append_row and append_rows. It mirrors the default= hook of json.dumps, so gspread keeps ownership of encoding while users handle types the stdlib cannot serialize on its own (e.g. datetime, Decimal). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
Closes #1556.
Adds the ability to configure a custom JSON serializer for request bodies, so users can write values that the standard library
jsonmodule cannot encode by default (e.g.datetime,Decimal) without pre-stringifying their data.Pass
Noneto restore the default behavior.Design
The issue suggested adding a
serializerargument at theWorksheet.update/batch_updatelevel. I went with configuring it once on the client instead, for a few reasons:HTTPClient.request()rather than threading a parameter through several method signatures, so it's smaller and less likely to break existing write paths.update,batch_update, etc.).set_timeoutpattern (setter on bothHTTPClientandClient), so it fits the codebase's conventions.If you'd prefer a per-call argument, that can be added on top of this foundation later. Happy to adjust the API in review.
Implementation
SerializerTypealias + optionalself.serializer(defaults toNone).HTTPClient.set_serializer()and aClient.set_serializer()forwarder.HTTPClient.request(), when a serializer is set and there is a JSON body, the body is serialized and sent asdata=with aContent-Type: application/jsonheader instead of viajson=. Without a serializer, behavior is unchanged.Backward compatibility
Fully backward compatible.
serializerdefaults toNone, and the existingjson=code path is untouched when no serializer is set.Tests
New
tests/http_client_test.py(mocked session, no cassette needed since this is request-layer logic):test_default_uses_json_kwarg: no serializer, so the body still goes out viajson=(proves unchanged behavior).test_custom_serializer_uses_data_and_header: serializer set, so the body is serialized intodata=,jsonis nulled, and the JSON content-type header is added.test_custom_serializer_handles_non_native_types: adatetime.date(which stdlibjson.dumpsrejects) is written successfully.Also manually verified end-to-end against the live Sheets API: without a serializer a
datewrite raisesTypeError, and with one the value lands correctly in the cell.Docs
Added a short "Using a Custom JSON Serializer" section to the user guide.
Notes
.. versionadded::directive since I don't know the target release. Happy to add6.3.0(or whatever you prefer) on request.