Skip to content

[AppService] BREAKING CHANGE: az webapp up: add optional params (os & runtime) and updated runtimes #15522

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
Merged
qwordy merged 7 commits into from
Oct 21, 2020
Merged

[AppService] BREAKING CHANGE: az webapp up: add optional params (os & runtime) and updated runtimes #15522

qwordy merged 7 commits into from
Oct 21, 2020

Conversation

@calvinsID
Copy link
Contributor

@calvinsID calvinsID commented Oct 14, 2020 •
edited
Loading

Description

  • Add optional parameters (os, runtime) to override az webapp up os & runtime detection
  • Updated some webapp runtime values (in WebappRuntimeStacks.json)

Testing Guide

  • az webapp up --resource-group MyRg --name MyWebapp --runtime "node|12-LTS" --os "windows"

This checklist is used to make sure that common guidelines for a pull request are followed.

@panchagnula panchagnula added this to the S177 milestone Oct 14, 2020
@panchagnula panchagnula added the Web Apps az webapp label Oct 14, 2020
@calvinsID calvinsID force-pushed the webapp-up-os-runtime branch 5 times, most recently from 0be6a7a to fc3d6d9 Compare October 16, 2020 00:17
@calvinsID calvinsID marked this pull request as ready for review October 16, 2020 00:19
@calvinsID calvinsID requested a review from panchagnula October 16, 2020 00:19
@calvinsID calvinsID force-pushed the webapp-up-os-runtime branch from fc3d6d9 to 559c0e8 Compare October 16, 2020 00:24
@yungezz
Copy link
Member

yungezz commented Oct 16, 2020

hi @qwordy could you pls help to review? thanks

@Azure Azure deleted a comment from yonzhan Oct 16, 2020
@calvinsID calvinsID force-pushed the webapp-up-os-runtime branch from 559c0e8 to eb158a1 Compare October 16, 2020 01:00
panchagnula
panchagnula reviewed Oct 16, 2020
View reviewed changes
@calvinsID calvinsID force-pushed the webapp-up-os-runtime branch from eb158a1 to 0ad5bc8 Compare October 16, 2020 18:13
@panchagnula
Copy link
Contributor

panchagnula commented Oct 16, 2020

@calvinsID I am sure you already validated - all the existing scenarios.
What happens with for an existing Windows app, if a user run --os to Linux - hoping we will fail gracefully
what happens if an existing app is runtime is different from what they are using in --runtime, do we just update the runtime - & if so do we need any validation or error handling like some python versions are supported on a specific OS only or does API throw useful errors here?

@calvinsID
Copy link
Contributor Author

calvinsID commented Oct 16, 2020

@panchagnula Yes lemme make sure I cover those cases in the webapp_up tests as well

@qwordy
Copy link
Member

qwordy commented Oct 19, 2020

@panchagnula Yes lemme make sure I cover those cases in the webapp_up tests as well

Ready to merge?

@calvinsID
Copy link
Contributor Author

calvinsID commented Oct 19, 2020

@panchagnula Yes lemme make sure I cover those cases in the webapp_up tests as well

Ready to merge?

I'm gonna add a few more test cases first

panchagnula
panchagnula approved these changes Oct 20, 2020
View reviewed changes
Copy link
Contributor

@panchagnula panchagnula left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for testing

@calvinsID calvinsID force-pushed the webapp-up-os-runtime branch from bfc3662 to a21b33e Compare October 21, 2020 00:07
@calvinsID calvinsID changed the title [AppService] az webapp up: add optional params os, runtime [AppService] BREAKING CHANGE: az webapp up: add optional params (os & runtime) and updated runtimes Oct 21, 2020
@qwordy qwordy merged commit ddeaff1 into Azure:dev Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Juliehzl Juliehzl Awaiting requested review from Juliehzl

2 more reviewers

@qwordy qwordy qwordy approved these changes

@panchagnula panchagnula panchagnula approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@qwordy qwordy

Labels

Web Apps az webapp

Projects

None yet

Milestone

S177

Development

Successfully merging this pull request may close these issues.

4 participants

@calvinsID @yungezz @panchagnula @qwordy