Skip to content

feat(runs): prune empty setting messages before storing - #7937

Merged
pingsutw merged 2 commits into
flyteorg:mainfrom
davidlin20dev:feat/settings-prune
Aug 31, 2026
Merged

pingsutw merged 2 commits into
flyteorg:mainfrom
davidlin20dev:feat/settings-prune

Conversation

@davidlin20dev

Copy link
Copy Markdown
Contributor

Tracking issue

Related to #7775. This closes task 2.1 (proto to protojson transformers), in a smaller form than the RFC specced. Related to #7932.

Why are the changes needed?

Task 2.1 asked for two transformers around stored protojson: Hydrate on read and PruneInherited on write. Most of that job turned out to be built into the serialization layer already:

  • Hydrate is a no-op. INHERIT is the proto3 zero value, and the nil-safe getters return it for absent fields, so a field missing from stored JSON already reads back as INHERIT. So there is nothing to implement.
  • Prune is mostly free. protojson.Marshal already omits zero scalars and nil messages. The one gap: a client can spell INHERIT as an empty message ("defaultQueue": {}), and that form is stored verbatim and echoed back by GetSettingsForEdit.

The empty-message form changes no behavior (both forms read back identically), but it makes stored bytes depend on client phrasing. The Phase 2 exit criteria pins exact response bytes against settings_customer_flow.md, which needs one canonical stored form. This PR adds the missing piece and drops the rest of 2.1.

What changes were proposed in this pull request?

pruneSettings in runs/service/settings_prune.go, called by CreateSettings and UpdateSettings right before marshaling:

  • Walks the message generically with protoreflect and clears every message field with nothing populated inside it. New Settings fields are covered automatically.
  • Children are pruned before their parent is judged, so a parent emptied by pruning is removed too.
  • UNSET survives: its state field is populated, so only truly empty messages are removed.
  • Side effect worth noting: Create and Update echo the request settings in their responses, so after pruning the echo matches the stored bytes exactly.

How was this patch tested?

  • Table tests in settings_prune_test.go: removal at every nesting depth, an empty map wrapper, UNSET kept, a value kept next to an empty sibling, clean settings unchanged, and a nil guard.
  • Assertions use proto.Equal, since protojson output bytes are deliberately unstable.
  • Existing settings service tests pass unchanged, so no read path observes the difference.

Labels

  • changed

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Signed-off-by: davidlin20dev <davidlin20.dev@gmail.com>
Signed-off-by: davidlin20dev <davidlin20.dev@gmail.com>
@pingsutw
pingsutw merged commit 711cceb into flyteorg:main Aug 31, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants