Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional Field Incorrectly Generated for Required Parameter in Python Codegen #329

Open
rangerthegood opened this issue Oct 26, 2024 · 2 comments · May be fixed by #330
Open

Optional Field Incorrectly Generated for Required Parameter in Python Codegen #329

rangerthegood opened this issue Oct 26, 2024 · 2 comments · May be fixed by #330

Comments

@rangerthegood
Copy link
Contributor

Steps to reproduce:

cd smithy-python/codegen
./gradlew smithy-python-codegen-test:build

This produces the file: smithy-python/codegen/smithy-python-codegen-test/build/smithyprojections/smithy-python-codegen-test/source/python-client-codegen/weather/models.py.

Looking at the GetCityInput structure you will see showing that city_id is not required because it allows None.

class GetCityInput:

    city_id: str | None = None

The smithy model for GetCityInput shows:

@http(method: "GET", uri: "/cities/{cityId}")
operation GetCity {
    input := {
        // "cityId" provides the identifier for the resource and
        // has to be marked as required.
        @required
        @httpLabel
        cityId: CityId
    }

The determination to see if a field is required is done in StructureGenerator.java.

        var required = new ArrayList<MemberShape>();
        var optional = new ArrayList<MemberShape>();
        var index = NullableIndex.of(context.model());
        for (MemberShape member: shape.members()) {
            if (index.isMemberNullable(member) || member.hasTrait(DefaultTrait.class)) {
                optional.add(member);
            } else {
                required.add(member);
            }
        }

isMemberNullable is returning True for the cityId, and therefore is marked as optional.

I'll PR adding an additional check to see if the RequiredTrait.class is set on the member.

When that is in place the structure then becomes:

class GetCityInput:

    city_id: str

Or maybe this is by design? #182

@rangerthegood rangerthegood linked a pull request Oct 26, 2024 that will close this issue
@JordonPhillips
Copy link
Contributor

This is by design. When you use the inline syntax for creating an input structure (:=), the structure created has the @input trait, which changes nullability for the sake of backwards compatibility.

@rangerthegood
Copy link
Contributor Author

Here is another case where my data is considered optional.

@idempotent
@http(method: "PUT", uri: "/car/{id}")
operation UpdateCar {
    input: UpdateCarInput
    output: UpdateCarOutput
}

structure UpdateCarlInput {
    @required
    @httpLabel
    id: String
...
}

The id would be optional in the generated Python code. Maybe the httpLabel makes it nullable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants