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

WIP: Truncate objects the same way we truncate other messages #12322

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ssbarnea
Copy link
Member

@ssbarnea ssbarnea commented May 14, 2024

Fixes: #12307
Related: #6682

@ssbarnea ssbarnea added the type: bug problem that needs to be addressed label May 14, 2024
Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Hi @ssbarnea, thanks for the PR!

Btw sorry about the frustration this seems to be causing, I intended to tackle this when I had the time, but free time has eluded me in the past week.

About the solution, while it would work, seems we already consider verbosity when deciding the max size used by saferepr here:

def _get_maxsize_for_saferepr(config: Optional[Config]) -> Optional[int]:
"""Get `maxsize` configuration for saferepr based on the given config object."""
if config is None:
verbosity = 0
else:
verbosity = config.get_verbosity(Config.VERBOSITY_ASSERTIONS)
if verbosity >= 2:
return None
if verbosity >= 1:
return DEFAULT_REPR_MAX_SIZE * 10
return DEFAULT_REPR_MAX_SIZE

So ideally we should just use that function in order to honor that. Seems to me it could be a matter of just replacing saferepr by _saferepr here:

obj = saferepr(obj)

If that works, I think we can even consider this a bug fix, as it seems this should be using _saferepr all along, being an oversight.

@ssbarnea ssbarnea changed the title Allow override of DEFAULT_REPR_MAX_SIZE Truncate objects the same way we truncate other messages May 15, 2024
@ssbarnea ssbarnea self-assigned this May 15, 2024
@ssbarnea ssbarnea marked this pull request as draft May 15, 2024 12:18
@ssbarnea ssbarnea changed the title Truncate objects the same way we truncate other messages WIP: Truncate objects the same way we truncate other messages May 15, 2024
@ssbarnea
Copy link
Member Author

ssbarnea commented May 15, 2024

@nicoddemus Apparently that change does not fix it and now we have a separate bug report for this problem. I tried to find the root cause but run out of time for today. I will first add a reproducing test for as we clearly do not want this to regress in the future.

@nicoddemus
Copy link
Member

Thanks @ssbarnea!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug problem that needs to be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extra info in assertion is truncated even with -vv
2 participants