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

Use configured message limit when creating serializers #634

Merged
merged 1 commit into from
Jul 31, 2018
Merged

Use configured message limit when creating serializers #634

merged 1 commit into from
Jul 31, 2018

Conversation

Wirone
Copy link
Contributor

@Wirone Wirone commented Jul 27, 2018

When message_limit is set in Raven client options, serializers should use this value instead of default message limit from RAVEN_Client::MESSAGE_LIMIT.

When `message_limit` is set in Raven client options, serializers should use this value instead of default message limit from `RAVEN_Client::MESSAGE_LIMIT`.
Copy link
Collaborator

@stayallive stayallive left a comment

Choose a reason for hiding this comment

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

Yes, yes they should! Thanks 😄

@Jean85
Copy link
Collaborator

Jean85 commented Jul 31, 2018

Do we need to fix the same thing on 2.0? @ste93cry ?

@stayallive stayallive merged commit 4c4ca2c into getsentry:master Jul 31, 2018
@ste93cry
Copy link
Collaborator

There is not anymore a message_limit configuration option in 2.0: I'm not sure about why i've removed it, maybe because the limit was hardcoded anyway in Sentry server anyway or because I've decided to cleanup a bit the amount of options we had. Anyway since 2.0 the limit is enforced in the MessageInterfaceMiddleware middleware as you can see below, so the best thing users can do to change it is to create their own middleware. I would like to avoid at all costs to add more options than necessary like in 1.x where we have a huge amount of them

$event = $event->withMessage(substr($message, 0, Client::MESSAGE_MAX_LENGTH_LIMIT), $messageParams);

@Wirone Wirone deleted the patch-1 branch August 1, 2018 13:23
@Jean85 Jean85 mentioned this pull request Aug 3, 2018
11 tasks
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.

4 participants