FormulaAndFunctionQueryDefinition group_by is list of objects, not dicts#3768
FormulaAndFunctionQueryDefinition group_by is list of objects, not dicts#3768ddadamhooper wants to merge 4 commits into
Conversation
Two related deserialization bugs surfaced when parsing the topology_map and geomap dashboard widgets: - ModelNormal.set_attribute only propagated _unparsed from list elements that were UnparsedObject instances, missing nested OpenApiModel instances that were themselves _unparsed. A topology_map request whose query carried a mismatched data_source enum could ambiguously satisfy both oneOf variants without being flagged. - allows_single_value_input skipped list oneOf branches, returning False for a ModelComposed such as FormulaAndFunctionEventQueryGroupByConfig (oneOf of a group-by list or a flat fields object). deserialize_model then splatted a list input into positional args, crashing ModelComposed.__init__; the TypeError was swallowed during matching and masked the failure once _unparsed propagation was fixed. Propagate _unparsed from nested models in lists, and treat a list oneOf branch as allowing single-value input so list inputs route through get_oneof_instance. Applied to both the generated model_utils.py and the generator template.
|
@codex review |
ddadamhooper
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0bad42583
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
/merge |
|
View all feedbacks in Devflow UI.
This pull request is not mergeable according to GitHub. Common reasons include pending required checks, missing approvals, or merge conflicts — but it could also be blocked by other repository rules or settings.
adam.hooper@datadoghq.com unqueued this merge request |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 326bb385fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
gh-worker-dd-devflow-36fce6
Bot
added
mergequeue-status: removed
and removed
mergequeue-status: waiting
labels
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 85501f8615
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ] | ||
| else: | ||
| list_oneof_instance = [oneof_class(**arg, **constant_kwargs) for arg in model_arg] | ||
| if not any(item._unparsed for item in list_oneof_instance): |
There was a problem hiding this comment.
When model_arg is an empty array, list_oneof_instance is also empty and any(...) is false, so this records the array branch even though non-list branches are still evaluated with empty model_kwargs. For datadog_api_client.v2.model.any_value.AnyValue (AnyValueObject plus [AnyValueItem]), deserializing a valid JSON value [] now records both the object and array matches and falls through to UnparsedObject, marking fields that use AnyValue as unparsed. Skip non-list branches for list payloads or special-case empty arrays so the array branch can be returned without ambiguity.
Useful? React with 👍 / 👎.
2 participants
Fixes bugs uncovered by DataDog/datadog-api-spec#5974 Test branch datadog-api-spec/test/adam.hooper/topology-widget-freshen-2
Behavior change
Solved two small parsing bugs:
oneOfmatched both branchesSpotted during
TopologyMapWidgetDefinitionchange.After the schema change,
TopologyMapWidgetDefinitionDataStreamshas an attributerequestswith each child aTopologyRequestDataStreams. AndTopologyMapWidgetDefinitionServiceMaphas an attributerequestswith each childTopologyRequestServiceMap.When parsing a service-map widget, we test both branches:
TopologyMapWidgetDefinitionServiceMap=> succeeds, because itsrequestscontains a validTopologyRequestServiceMapTopologyMapWidgetDefinitionDataStreams=> succeeds in error, because itsrequestscontains an invalidTopologyRequestDataStreamsbut that_unparsedwasn't propagated to the list.Fix: propagate
_unparsedso the list is marked unparsed.Formula Events group-by lists failed to parse
Existing bug -- this is a breaking client-API change! It fixes another breaking client-API changed introduced in #3230.
#3230 (March 2026) introduced a regression: Formula Events group-by lists failed to parse.
What we wanted:
What we shipped:
This is because the parser wouldn't accept
oneOfwhere one branch is a list.But we didn't see this error because we weren't propagating
_unparsedcorrectly.Fix:
deserialize_model(): accept a list as a validoneOfchild.TL;DR
Customer-facing change in this PR: parsing Dashboards API responses, formula group-by clauses WERE list-of-dicts, and are NOW list-of-objects. This fixes a regression.