Skip to content
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

👌 Add configuration for recursion limit in workers #6044

Merged
merged 6 commits into from
Jun 17, 2023

Conversation

chrisjsewell
Copy link
Member

Add daemon.recursion_limit config option which, only if specifically set by the user, will change the recursion limit within the worker Python interpreter.

Addresses #4876

As specified, currently no change is made without a user specifically setting the config option, meaning that the default 1000 limit will be used.

It could be an option to set a specific (higher) default for this option.
This has precedent, for example if one opens an IPython terminal session you will find the limit actually set at 3000, since jedi resets the limit: https://github.com/davidhalter/jedi/blob/a28bd24befa4331101d151df10445367de8df16a/jedi/api/__init__.py#L48
The Python 1000 default limit is quite conservative, and given IPython doesn't have an issue with 3000, this should be fine, although setting it too high may lead to stack overflow errors, since Python doesn't handle recursion in a particular efficiently (memory-wise).

@sphuber
Copy link
Contributor

sphuber commented Jun 15, 2023

I would vote for actually increasing the default since we know that for many use cases this limit will anyway have to be increased and we also know from it being used by ipython that it should be a safe and reasonable default. Agree?

@sphuber sphuber added this to the v2.4.0 milestone Jun 15, 2023
@sphuber
Copy link
Contributor

sphuber commented Jun 16, 2023

@chrisjsewell you want to update the default to 3000? Then we can merge this

@sphuber sphuber self-requested a review June 16, 2023 18:52
@sphuber sphuber merged commit 226159f into aiidateam:main Jun 17, 2023
12 checks passed
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.

None yet

2 participants