Skip to content

Conversation

MintsInc
Copy link
Member

@MintsInc MintsInc commented Oct 10, 2025

Context

PR https://github.com/DataDog/datadog-api-spec/pull/4191 introduced a new model CustomAttributeValuesUnion with a oneOf schema containing primitive list types:

oneOf: [str, [str], float, [float]]

This caused test failures in the generated Python client (#2759):

The issue was that the generator tried to instantiate primitive types using dict unpacking (str(**arg, **kwargs)), which fails with a TypeError.

This is because of two bugs in the model_utils.j2 template:

  1. get_oneof_instance function: When handling list types in oneOf schemas, the code didn't distinguish between:

    • Lists of primitives (e.g., [str], [float])
    • Lists of ModelSimple objects
    • Lists of complex objects (ModelNormal, ModelComposed)

    That's because checking cls in PRIMITIVE_TYPES doesn't catch a list of string for example.

    It treated all lists the same way, using dict unpacking which only works for complex objects.

  2. composed_model_input_classes function: Called issubclass([str], ModelSimple) which fails with TypeError: issubclass() arg 1 must be a class when the oneOf schema contains list types like [str].

Changes

Updated .generator/src/generator/templates/model_utils.j2 with two fixes:

1. Type Guard in composed_model_input_classes (lines 73-92)

Added a guard to handle list types before calling issubclass:

if isinstance(cls, list):
    return [cls]

This prevents TypeError: issubclass() arg 1 must be a class when encountering list types like [str] or [float] in oneOf schemas.

2. Three-Way List Handling in get_oneof_instance (lines 1558-1602)

Distinguished between three types of list contents:

  • Lists of primitives ([str], [float]): Use validate_and_convert_types to properly handle primitive values
  • Lists of ModelSimple: Use single-arg constructor ModelSimple(arg)
  • Lists of complex objects: Use dict unpacking (existing behavior preserved)

Tests

I cherry-picked the changes from this PR onto #2759 and ran the tests (I unskipped the problematic test before)

Related Work

Similar issues were fixed in the Java client:

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@MintsInc MintsInc marked this pull request as ready for review October 10, 2025 10:27
@MintsInc MintsInc requested review from a team as code owners October 10, 2025 10:27
@MintsInc MintsInc force-pushed the ulysse.mavrocordatos/fix-oneof-primitive-list branch from 008536d to 0cc8913 Compare October 10, 2025 15:22
@MintsInc MintsInc requested a review from amaskara-dd October 10, 2025 15:22
@MintsInc MintsInc force-pushed the ulysse.mavrocordatos/fix-oneof-primitive-list branch from 0cc8913 to 59d3c55 Compare October 13, 2025 07:50
@MintsInc MintsInc force-pushed the ulysse.mavrocordatos/fix-oneof-primitive-list branch from 59d3c55 to 548f62d Compare October 15, 2025 07:57
Copy link
Contributor

@Kaycell Kaycell left a comment

Choose a reason for hiding this comment

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

This LGTM but if I'm correct we run test scenarios against these models? Can we add one with oneOf?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants