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.

Sign up for GitHub

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

Jump to bottom

Add container-azure-* templates #401

Merged
merged 9 commits into from
Oct 21, 2022
Merged

Add container-azure-* templates #401

merged 9 commits into from
Oct 21, 2022

Conversation

cnunciato
Copy link
Contributor

@cnunciato cnunciato commented Oct 11, 2022 •
edited
Loading

This change adds new templates for container-azure-{typescript,python,go,csharp}. Apps return JSON responses using common language-specific frameworks for easy extensibility. Excludes YAML for now, as docker:Image is not yet supported in YAML. (See pulumi/pulumi-docker#132.)

Fixes https://github.com/pulumi/devrel-team/issues/286.

@cnunciato cnunciato force-pushed the cnunciato/add-azure-container branch 2 times, most recently from 04dd2b8 to 9d8c643 Compare October 14, 2022 23:49
@cnunciato cnunciato requested a review from a team October 17, 2022 23:23
@cnunciato cnunciato marked this pull request as ready for review October 17, 2022 23:23
@cnunciato cnunciato force-pushed the cnunciato/add-azure-container branch from 31c7ed1 to b234179 Compare October 17, 2022 23:23
@cnunciato cnunciato requested a review from gthuang October 17, 2022 23:24
@cnunciato cnunciato force-pushed the cnunciato/add-azure-container branch 4 times, most recently from 5d060dc to 197ac7b Compare October 19, 2022 16:35
scottslowe
scottslowe approved these changes Oct 20, 2022
View reviewed changes
Copy link
Contributor

@scottslowe scottslowe left a comment

Choose a reason for hiding this comment

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

Hey @cnunciato, the C# and Go templates work perfectly but there is a "corner case" that causes Python and TypeScript to fail (related to Docker networking). I don't know that it's necessarily worth trying to address that (I suspect it would be rather rare) so I'll go ahead and approve the PR.

@cnunciato cnunciato force-pushed the cnunciato/add-azure-container branch from a413692 to fe9cbbe Compare October 20, 2022 18:29
@cnunciato cnunciato force-pushed the cnunciato/add-azure-container branch from ae19faa to 4a593de Compare October 20, 2022 22:26
@cnunciato
Copy link
Contributor Author

cnunciato commented Oct 21, 2022 •
edited
Loading

@scottslowe I wasn't able to get the emoji rendering to work reliably in all browsers (even after ensuring all app servers returned charset=utf-8), so I went ahead and removed them. 😢

@scottslowe
Copy link
Contributor

I was able to get them to render in Firefox, Chrome, and Safari (after changing the charset). Which browser didn't work for you?

@cnunciato
Copy link
Contributor Author

cnunciato commented Oct 21, 2022 •
edited
Loading

Go and TypeScript worked fine in all browsers, but C# and Python didn't work for me in either Chrome or Safari, sadly.

@cnunciato cnunciato merged commit b26e73a into master Oct 21, 2022
@pulumi-bot pulumi-bot deleted the cnunciato/add-azure-container branch October 21, 2022 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@scottslowe scottslowe scottslowe approved these changes

@gthuang gthuang Awaiting requested review from gthuang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cnunciato @scottslowe