Notifications#event_payload too long #4161
Comments
|
this is the repo checker complaining about rubygems having file conflict between 2.4 and 2.5. For comments there should be a limit IMO - but this limit shouldn't be the event_payload, agreed :) If @jberry-suse saw a '400 too much data in comments' instead of a 500, he would perhaps check his scripts :) |
|
So as I asked for in the original issue...what limit do I use? I added comment truncation that handles formatting and what not properly, but need the limit. Last I checked it was working, but as discussed in original thread the implementation was done such that the limit is likely to be inconsistent since it depends on what else ends up in the payload. I can just drop the limit by several thousand and cross my fingers. My $0.02 I would suggest picking a limit for comments that provides a margin for the notification mess and validate as such so a proper error is given and I'll use the same limit. |
|
Also, I can look in the logs, but after I made the truncation I read logs regularly for several weeks and never saw issues. It seems like the comments are still created just not the notifications? |
|
Don't you think that 64K of comment is too much - no matter if it ends up in a mail or not? |
|
most of the issues exploding now are 'there is some generic problem with 2 sets of packages' - the exact number of files conflicting isn't that interesting. IMO you're abusing the comment system there - and the build service should restrict comments to a saner level than 64K. |
|
@jberry-suse it's not about you. my mention was just meant informative :-) |
|
Mixed messages. :-) Unless the notification payload is increased I will need to decrease tool limit to one number or another. As to size of message, the goal was to provide feedback to user from the tool and the tool provides the message. Given that the tool does not provide structured output short of simply truncating the message it is a bit of work to condense it effectively. So as goes the rule the "simplest thing that can work" was done first as the primary goal was submitters actually being able to see the messages directly which was not previously possible and the other goals of the re-write which are not relevant to this discussion. At this point we could look at parsing the text and condensing or just truncating to a shorter length. Alternatively, having a better place to store the data might be useful, but the comments being used for the purpose has become the norm for these tools, as you know, for lack of any better mechanism. One issue with shortening is that problems may end up being fixed in repeated batches due to not being able to see the full output. In some cases sure it is just a list of files between two packages, but in other large stagings it can be a series or more complex messages that involve multiple submissions. If the message is cut too short not everyone will see the messages effecting their package in the first round (or second, or third...). Obviously, this can still happen with the current max. Since the comments are always removed since stagings are re-used it is not as if these comments are being kept long-term and should only require storage of active set which does not increase over time. Could additionally, upload the full message to pastebin, but most pastebins have a similar limit. So short of having a proper place to host the full output, or perhaps using one of various storage providers like S3 or somesuch comments are clearly the only real solution at the moment. |
- Add shorten_payload_if_necessary method in Notification model to be sure that a payload is not bigger than the field size in database. - This should fix openSUSE#4161.
- Add shorten_payload_if_necessary method in Notification model to be sure that a payload is not bigger than the field size in database. - This should fix openSUSE#4161.
- Add shorten_payload_if_necessary method in Notification model to be sure that a payload is not bigger than the field size in database. - This should fix openSUSE#4161. Mobprogrammed with @mdeniz and @DavidKang.
- Add shorten_payload_if_necessary method in Notification model to ensure that a payload is not bigger than the field size in database. - Modify the notifiction factory to allow passing parameters to initialize. - This should fix openSUSE#4161. Mobprogrammed with @mdeniz and @DavidKang.
- Add shorten_payload_if_necessary method in Notification model to ensure that a payload is not bigger than the field size in database. - Modify the notifiction factory to allow passing parameters to initialize. - This should fix openSUSE#4161. Mobprogrammed with @mdeniz and @DavidKang.
- Add shorten_payload_if_necessary method in Notification model to ensure that a payload is not bigger than the field size in database. - Modify the notifiction factory to allow passing parameters to initialize. - This should fix openSUSE#4161. Mobprogrammed with @mdeniz and @DavidKang.
- Add shorten_payload_if_necessary method in Notification model to ensure that a payload is not bigger than the field size in database. - Modify the notification factory to allow passing parameters to initialize. - This should fix openSUSE#4161. Mobprogrammed with @mdeniz and @DavidKang.
SendEventEmailsJob is creating notifications from certain events. As part of this the serialized event payload is copied to the 'event_payload' attribute. Both models were using different serialization libraries (Yajl::Encoder for events, ActiveSupport::JSON.encode for notifications), which could cause the stored data format to differ, eg. active support encodes certains chars differently. In general this was not causing any trouble, because the decoded data would be the same. But it could happen that the payload of the notification is bigger than the original event payload and exceeds the size limit of that field. In such a case the notification creation failed with a ActiveRecord::ValueTooLong error. By using the same serialization method in both places, we avoid this from happening. Since notifications are created from events, and the event payload length is already sanitized based on that serialization method used by the event model, we pick the Yajl::Encoder. Fixes openSUSE#3638 openSUSE#4161
SendEventEmailsJob is creating notifications from certain events. As part of this the serialized event payload is copied to the 'event_payload' attribute. Both models were using different serialization libraries (Yajl::Encoder for events, ActiveSupport::JSON.encode for notifications), which could cause the stored data format to differ, eg. active support encodes certains chars differently. In general this was not causing any trouble, because the decoded data would be the same. But it could happen that the payload of the notification is bigger than the original event payload and exceeds the size limit of that field. In such a case the notification creation failed with a ActiveRecord::ValueTooLong error. By using the same serialization method in both places, we avoid this from happening. Since notifications are created from events, and the event payload length is already sanitized based on that serialization method used by the event model, we pick the Yajl::Encoder. Fixes openSUSE#3638 openSUSE#4161
SendEventEmailsJob is creating notifications from certain events. As part of this the serialized event payload is copied to the 'event_payload' attribute. Both models were using different serialization libraries (Yajl::Encoder for events, ActiveSupport::JSON.encode for notifications), which could cause the stored data format to differ, eg. active support encodes certains chars differently. In general this was not causing any trouble, because the decoded data would be the same. But it could happen that the payload of the notification is bigger than the original event payload and exceeds the size limit of that field. In such a case the notification creation failed with a ActiveRecord::ValueTooLong error. By using the same serialization method in both places, we avoid this from happening. Since notifications are created from events, and the event payload length is already sanitized based on that serialization method used by the event model, we pick the Yajl::Encoder. Fixes openSUSE#3638 openSUSE#4161
SendEventEmailsJob is creating notifications from certain events. As part of this the serialized event payload is copied to the 'event_payload' attribute. Both models were using different serialization libraries (Yajl::Encoder for events, ActiveSupport::JSON.encode for notifications), which could cause the stored data format to differ, eg. active support encodes certains chars differently. In general this was not causing any trouble, because the decoded data would be the same. But it could happen that the payload of the notification is bigger than the original event payload and exceeds the size limit of that field. In such a case the notification creation failed with a ActiveRecord::ValueTooLong error. By using the same serialization method in both places, we avoid this from happening. Since notifications are created from events, and the event payload length is already sanitized based on that serialization method used by the event model, we pick the Yajl::Encoder. Fixes openSUSE#3638 openSUSE#4161
eduardoj
changed the title
|
This should be done... closing! |
There are over 400 exceptions in errbit about this. All for the
openSUSE:Factory:Stagingprojects.@jberry-suse looks like some script is passing too much data again.
I still think this should be medium text as mentioned in #3638
The text was updated successfully, but these errors were encountered: