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

Bump cadl ranch 0.37.1 #2815

Merged
merged 10 commits into from
Sep 9, 2024
Merged

Bump cadl ranch 0.37.1 #2815

merged 10 commits into from
Sep 9, 2024

Conversation

msyyc
Copy link
Member

@msyyc msyyc commented Sep 5, 2024

No description provided.

// for `temperature: HttpPart<{@body body: float64, @header contentType: "text/plain"}>`, the real type is float64
sourceType = body.type;
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

For temperature: HttpPart<{@body body: float64, @header contentType: "text/plain"}>, TCGC provide model of {@body body: float64, @header contentType: "text/plain"} while python emitter actually only need float64 so we need to parse it.

Copy link
Contributor

Choose a reason for hiding this comment

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

is it bad that we have contentType? I'm thinking we still just pass it through. But I'm good either way

Copy link
Member Author

Choose a reason for hiding this comment

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

HttpPart permit us to define more specific info for this part. In this case @header contentType: "text/plain" means SDK shall serialize the float64 as plain string instead of json string. Although both actually send same content, JS owner thought we shall add the specific info in the cadl-ranch case then I just added it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I raise an issue Azure/typespec-azure#1488. If all languages agree Azure/typespec-azure#1488 (comment), TCGC could do the parse logic so that emitters don't need the duplicated work any more.

return (
self.type.is_form_data
or bool(self.entries)
or ("multipart/form-data" in self.content_types and self.code_model.options["from_typespec"])
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we need to check if it's "from_typespec". We should also remove this option and deal with the differences earlier on in the pipeline

Copy link
Member Author

Choose a reason for hiding this comment

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

Without from_typespec, there will be diff for legacy code under autorest.python folder. Since multipart is mainly for typespec design, I think we had better keep legacy same as before.

// for `temperature: HttpPart<{@body body: float64, @header contentType: "text/plain"}>`, the real type is float64
sourceType = body.type;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is it bad that we have contentType? I'm thinking we still just pass it through. But I'm good either way

@msyyc msyyc marked this pull request as ready for review September 6, 2024 02:22
@msyyc msyyc requested a review from tadelesh as a code owner September 6, 2024 02:22
@msyyc msyyc merged commit 9cac616 into main Sep 9, 2024
15 checks passed
@msyyc msyyc deleted the bump-cadl-ranch-2024 branch September 9, 2024 04:45
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 this pull request may close these issues.

2 participants