Skip to content

[Automation] Generate SDK based on TypeSpec 0.14.1#39109

Closed
azure-sdk wants to merge 7 commits into
Azure:mainfrom
azure-sdk:typespec-java-generation-3565631
Closed

[Automation] Generate SDK based on TypeSpec 0.14.1#39109
azure-sdk wants to merge 7 commits into
Azure:mainfrom
azure-sdk:typespec-java-generation-3565631

Conversation

@azure-sdk

@azure-sdk azure-sdk commented Mar 7, 2024

Copy link
Copy Markdown
Collaborator

[Automation] Generate SDK based on TypeSpec 0.14.1

azure-communication-jobrouter

  • compile, it uses json-merge-patch + polymorphism
  • not able to access serializeAsJsonMergePatch in different package

azure-communication-messages

  • compile, base class been customized to be abstract, commented
  • setKind not accessible in different package, commented

contentsafety

  • revapi, removed annotation, should be expected

easm

  • test, request/response body diff, may need help from service dev to record

healthinsights

  • compile, bug in serializaion, commented

I've already skipped azure-ai-openai. Its customization targets Jackson.

Fix on azure-communication-jobrouter probably takes time.

The issue on azure-communication-messages and healthinsights may not be easy.

Should we temporarily skip these libs and check in 0.14.1, then fix them later (may choose either set the flag to false in tspconfig; or temporary skip the codegen CI).


List of issues put to #38569 (comment)

@azure-sdk

azure-sdk commented Mar 7, 2024

Copy link
Copy Markdown
Collaborator Author

super(resourceType);
setResourceType("Fhir_R4_DomainResource");

@weidongxu-microsoft weidongxu-microsoft Mar 7, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

setResourceType not generated in class.

The class is a bit unusual. It has a base class of FhirR4Resource, but only this class have discriminator property.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for finding this. Looks like there is an edge case with code generation where the discriminator is a required property.

reader.skipChildren();
}
}
MessageTemplateItem deserializedMessageTemplateItem = new MessageTemplateItem(language, status);

@weidongxu-microsoft weidongxu-microsoft Mar 7, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The class was customized to abstract by service.

What can we do via customization to skip this fromJsonKnownDiscriminator?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The method can be deleted with customizations, but I think a better option would be allowing the type to be instantiated as Jackson had to be handling this before (likely by creating anonymous implementations)

super(language, status);
setKind(CommunicationMessagesChannel.WHATS_APP);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

setKind not accessible in a different pacakge

@weidongxu-microsoft weidongxu-microsoft Mar 7, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a problem not relate to stream-style-serialization.

Model by Jackson would have the setKind method as well (as the impl changed), and it would have the same problem. Azure/autorest.java#2574

As this case, our 0.14.x without bug fix would block service.

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.

3 participants