Conversation
CodSpeed Performance ReportMerging this PR will not alter performanceComparing Summary
Footnotes |
jakekaplan
requested review from
chrisguidry,
cicdw,
desertaxle and
zzstoatzz
as code owners
src/prefect/events/utilities.py
Outdated
| if event_obj.size_bytes > max_size: | ||
| raise EventTooLarge(event_obj.size_bytes, max_size) | ||
| if behavior == "error": | ||
| raise EventTooLarge(event_obj.size_bytes, max_size) |
There was a problem hiding this comment.
I think we should include a blurb about how to change this to a warning by updating the maximum_size_bytes_behavior setting.
There was a problem hiding this comment.
good call, added!
Co-authored-by: Alex Streed <desertaxle@users.noreply.github.com>
desertaxle
left a comment
desertaxle
left a comment
There was a problem hiding this comment.
Sorry, I missed some stuff on my first review. Feel free to punt on retrieving the settings via the API, and I can address that in a separate PR.
| validation_alias=AliasChoices( | ||
| AliasPath("maximum_size_bytes_behavior"), | ||
| "prefect_server_events_maximum_size_bytes_behavior", | ||
| "prefect_events_maximum_size_bytes_behavior", | ||
| ), |
There was a problem hiding this comment.
Sorry I missed this in the first review, but you don't need these aliases for a new setting. Those are used for backward compatibility.
| validation_alias=AliasChoices( | |
| AliasPath("maximum_size_bytes_behavior"), | |
| "prefect_server_events_maximum_size_bytes_behavior", | |
| "prefect_events_maximum_size_bytes_behavior", | |
| ), |
| events_settings = get_current_settings().server.events | ||
| max_size = events_settings.maximum_size_bytes | ||
| behavior = events_settings.maximum_size_bytes_behavior |
There was a problem hiding this comment.
I know this was already like this, but I'd rather access server settings directly in the client since those are only supposed to control server behavior. Instead, we should read the server settings via the /admin/settings endpoint.
| ), | ||
| ) | ||
|
|
||
| maximum_size_bytes_behavior: Literal["error", "warn", "ignore"] = Field( |
There was a problem hiding this comment.
I just realized this is a server setting, but it should probably go in a new model in prefect/src/prefect/settings/models/events.py instead.
Oh no worries! Let me take a look at this again, I didn't realize this was a server setting originally |
|
This pull request is stale because it has been open 14 days with no activity. To keep this pull request open remove stale label or comment. |
2 participants
Event size limits exist on both OSS + Cloud. Before #19531, oversize events were accepted then silently dropped server‑side, which can hide missing automations/state updates and is hard to debug. Raising
EventTooLargefails fast and makes the problem visible so users can adjust payloads.That being said, this PR makes raising that error configurable so users can still continue to write their workflows as they have before. That should unblock users in the short‑term. However, longer‑term I believe we'll need to possibly investigate how handle oversized events (for example a task run event that has tens of thousands of task run dependencies).