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

Provide API createWebAppOnLinux to create web apps on linux #1963

Merged
merged 3 commits into from
Sep 11, 2018
Merged

Provide API createWebAppOnLinux to create web apps on linux #1963

merged 3 commits into from
Sep 11, 2018

Conversation

SummerSun
Copy link
Contributor

@SummerSun SummerSun commented Sep 6, 2018

Fix the issue #1971

Use help methods prepareWithCreate and prepareServicePlan to handle duplicate codes during creating a new Windows or Linux service plan.

Add new API createWebAppOnLinux.

@SummerSun SummerSun requested review from Eskibear and jdneo September 6, 2018 10:52

AppServicePlan.DefinitionStages.WithPricingTier withPricingTier;
String resourceGroup = model.getResourceGroup();
if (isCreatingNewResourceGroup) {
Copy link
Member

Choose a reason for hiding this comment

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

Move boolean isCreatingNewResourceGroup = model.isCreatingResGrp(); here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove the variable declaration since it is used only once, check model.isCreatingResGrp() directly

AppServicePlan.DefinitionStages.WithPricingTier withPricingTier;
String resourceGroup = model.getResourceGroup();
if (isCreatingNewResourceGroup) {
withPricingTier = withGroup.withNewResourceGroup(resourceGroup);
Copy link
Member

@jdneo jdneo Sep 10, 2018

Choose a reason for hiding this comment

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

Is it possible to check if the resource group exists or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will check and handle it during validation in next PR

return appWithGroup.withExistingResourceGroup(resourceGroup).withNewWindowsPlan(withCreatePlan);
}

private WebApp.DefinitionStages.WithDockerContainerImage withCreateNewLinuxServicePlan(
Copy link
Member

Choose a reason for hiding this comment

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

Duplicated codes compared with withCreateNewWindowsServicePlan().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use helper methods to handle the duplicated codes.

@@ -48,6 +50,8 @@
private String region = "";
private String pricing = "";
private JavaVersion jdkVersion = JavaVersion.JAVA_8_NEWEST;
private RuntimeStack linuxRuntime = RuntimeStack.TOMCAT_8_5_JRE8;
private OperatingSystem os;
Copy link
Member

Choose a reason for hiding this comment

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

Why not assign a default value for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Matthew-Dong Any suggestions for the default value? Linux or Windows?

Copy link
Member

@Eskibear Eskibear left a comment

Choose a reason for hiding this comment

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

I see every function throws Exception. Does SDK simply throws Exception, or do we swallow the type of exceptions?

@SummerSun
Copy link
Contributor Author

SummerSun commented Sep 11, 2018

@Eskibear createWebAppOnWindows -> withCreateNewWindowsServicePlan -> prepareWithCreate
And in the last function an exception will be thrown when the pricingtier is invalid.

final String[] tierSize = model.getPricing().split("_");
if (tierSize.length != 2) {
    throw new Exception("Cannot get valid price tier");
}

@Eskibear
Copy link
Member

Suggest to create an issue for using customized Error type instead of the general Exception.

@SummerSun SummerSun merged commit 3074d15 into microsoft:develop Sep 11, 2018
@SummerSun SummerSun deleted the mvp branch September 11, 2018 08:30
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.

3 participants