Skip to content

Fix a RecursionError that occurs with deeply-nested request bodies - #1423

Merged
kurtmckee merged 2 commits into
mainfrom
fix-recursion-errors
Sep 2, 2026
Merged

Fix a RecursionError that occurs with deeply-nested request bodies#1423
kurtmckee merged 2 commits into
mainfrom
fix-recursion-errors

Conversation

@kurtmckee

Copy link
Copy Markdown
Member

Note

This issue was discovered while testing a service release.

The first commit, which introduces a test that demonstrates the problem, takes over 7 seconds locally to eventually throw RecursionError.

The second commit, which eliminates recursion entirely, makes the test succeed in 0.04 seconds.

Fixed

  • Fix a RecursionError that occurs with deeply-nested request bodies.

@sirosen sirosen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this is exchanging a very small and simple bit of recursive code for a much larger body of work to "unroll the loop", I think it's appropriate that we make sure we're reshaping this into something which is

  1. Maximally readable
  2. Maximally performant

We're reducing readability (net-net), so we should put in the effort to get as much benefit from that trade as possible.

I opened #1424 with a benchmark that I used to validate my suggestions. I encourage its use here -- also feel free to merge or cherry-pick it into here and make further changes! -- to verify these changes:

  • use a deque
  • use comprehensions rather than iteratively building dicts and lists (even though this requires the addition of a new helper function)

When I measured on Python 3.12 and 3.14, I found rather dramatic improvements, especially in the worst-case type of testing.
I only explore a very small fraction of the possible space with the proposed microbenchmark, so it may be worth our time to enhance it further?

Comment thread src/globus_sdk/transport/representation_providers.py Outdated
Comment thread src/globus_sdk/transport/representation_providers.py
Comment thread src/globus_sdk/transport/representation_providers.py Outdated
@kurtmckee
kurtmckee force-pushed the fix-recursion-errors branch from a664337 to 3153849 Compare September 2, 2026 12:49
@kurtmckee
kurtmckee merged commit 7fc4c80 into main Sep 2, 2026
10 checks passed
@kurtmckee
kurtmckee deleted the fix-recursion-errors branch September 2, 2026 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants