-
Notifications
You must be signed in to change notification settings - Fork 55
feat: limit max number of items in collection to scan #430
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
feat: limit max number of items in collection to scan #430
Conversation
posthog-python Compliance ReportDate: 2026-02-11 12:16:57 UTC ✅ All Tests Passed!29/29 tests passed Capture Tests✅ 29/29 tests passed View Details
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 file reviewed, 1 comment
posthog/exception_utils.py
Outdated
| result = {} | ||
| for k, v in value.items(): | ||
| for i, (k, v) in enumerate(value.items()): | ||
| if i >= _MAX_COLLECTION_ITEMS_TO_SCAN: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think truncating object is confusing, It would be better to just return a redacted value imo, like CODE_VARIABLES_TOO_LONG_VALUE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about it and I thought that it's better to return something than not return anything, no?
We can add an info thing on frontend that some of data was truncated because object was too long or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we display something on the frontend that the object has been truncated yes, but as a first step showing redacted values might be easier. I think we want to prevent users having to understand why their object or list does not contain all the items.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Applied that change
Some collections may have big number of items to scan.
This PR limits it to 100.
Note: it doesn't limit to 100 scans per original variable but rather 100 scans per depth level. But it sounds reasonable. It's so we don't run out of limit on stupid scans with huge nested collections.