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 APPMAP_MAX_TIME #337

Merged
merged 3 commits into from
Jun 5, 2024
Merged

Add APPMAP_MAX_TIME #337

merged 3 commits into from
Jun 5, 2024

Conversation

apotterri
Copy link
Collaborator

Add APPMAP_MAX_TIME which allows the user to specify, in seconds, the maximum time a recording session should be allowed.

In addition, disable rendering of the schema associated with values (parameter, return types) when APPMAP_DISPLAY_PARAMS=false.

Finally, move the implementation of __reduce_ex__ from wrapt.wrappers.FunctionWrapper up to ObjectProxy. Our implementation, which is good enough for generating AppMaps, works for any instance of an ObjectProxy.

@kgilpin
Copy link
Contributor

kgilpin commented Jun 5, 2024

I'm curious if pytest and unittest also have a test timeout setting?

How does test timeout setting differ in practice from a max events setting?

Add APPMAP_MAX_TIME: allows the user to specify the maximum time (in
seconds) a recording can be in progress. If the session exceeds this
time, AppMapSessionTooLong will be raised.
When APPMAP_DISPLAY_PARAMS is false, disable rendering of the schema of
values (e.g. params, return types). A schema can be deeply nested (as in
matplotlib), which blows up memory usage and mapping time.
The implementation that was previously in FunctionWrapper should work
for all subclasses of ObjectProxy.
@apotterri
Copy link
Collaborator Author

pytest has a timeout plugin: https://pypi.org/project/pytest-timeout/ . As far as I can tell, unittest doesn't support a global timeout, it requires you to use something like https://pypi.org/project/timeout-decorator/ .

In both of those cases, the test is going to get aborted, and no AppMap is going to get generated. Using APPMAP_MAX_TIME means that we'll still get an AppMap, with a test status showing us why it failed.

Setting APPMAP_MAX_EVENTS wasn't sufficient for dealing with memory issues, because there are other things that can cause high memory utilization. This PR has a fix for one of them (disabling the generation of properties for hash-like objects), but I expect that are others as well.

@kgilpin
Copy link
Contributor

kgilpin commented Jun 5, 2024

Cool, thanks for the explanation.

@dustinbyrne dustinbyrne merged commit f4618b6 into master Jun 5, 2024
2 checks passed
@dustinbyrne dustinbyrne deleted the max-time_20240605 branch June 5, 2024 13:27
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.

3 participants