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

Users/hiyada/war deployment #6579

Merged
merged 8 commits into from
Mar 5, 2018
Merged

Users/hiyada/war deployment #6579

merged 8 commits into from
Mar 5, 2018

Conversation

hiyadav
Copy link
Contributor

@hiyadav hiyadav commented Mar 1, 2018

No description provided.

@vincent1173 vincent1173 added Area: Release Area: AzureAppService Label to monitor Azure App Service issues labels Mar 1, 2018
vincent1173
vincent1173 reviewed Mar 3, 2018
View reviewed changes
Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
return new Promise<void>((resolve, reject) => setTimeout(resolve, timeoutInSeconds * 1000));
}

async function CallDeployedWebApp(webAppName: string): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

use await appServiceUtility.pingApplication(); to call the targeted app.
Reference

Copy link
Contributor

Choose a reason for hiding this comment

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

remove this functuion.

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
@@ -1,6 +1,7 @@
import tl = require('vsts-task-lib/task');
Copy link
Contributor

Choose a reason for hiding this comment

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

bump-up task patch version.

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
return new Promise<boolean>((resolve, reject) => {
// Waiting for war to expand
var sleep = SleepFunction(10);
sleep.then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

use await instead of then

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
var taskParams: TaskParameters = TaskParametersUtility.getParameters();
var azureEndpoint: AzureEndpoint = await new AzureRMEndpoint(taskParams.connectedServiceName).getEndpoint();
var virtualApplicationPath: string;
console.log(tl.loc('GotconnectiondetailsforazureRMWebApp0', taskParams.WebAppName));
if(!taskParams.DeployToSlotFlag) {
if (!taskParams.DeployToSlotFlag) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Remove spacing (to maintain consistency)

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
taskParams.ExcludeFilesFromAppDataFlag, taskParams.TakeAppOfflineFlag, taskParams.VirtualApplication, taskParams.SetParametersFile,
taskParams.AdditionalArguments, isFolderBasedDeployment, taskParams.UseWebDeploy);
// this is being done temporarily, to tackle war files not getting expanded (only for the first deployment to the web app) when done using MSDeploy.
if (webPackage.toString().toLowerCase().endsWith('.war')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a separate file under operationsFolder. (say MSdeployOperations.ts) and place your logic there.

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
@@ -1,6 +1,7 @@
import tl = require('vsts-task-lib/task');
import path = require('path');
import fs = require('fs');
import http = require('http');
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this module.

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
async function HasWarExpandedSuccessfully(kuduService, filesBeforeDeployment, webAppName: string, packageName: string): Promise<boolean> {
return new Promise<boolean>((resolve, reject) => {
// Waiting for war to expand
var sleep = SleepFunction(10);
Copy link
Contributor

Choose a reason for hiding this comment

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

you can import the sleep function from Webclient.ts., I suppose.

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
sleep.then(() => {
// do a get call on the target web app.
var callWebApp = CallDeployedWebApp(webAppName);
callWebApp.then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

use await instead of then/catch.

Tasks/AzureRmWebAppDeployment/azurermwebappdeployment.ts Outdated
var callWebApp = CallDeployedWebApp(webAppName);
callWebApp.then(() => {
var filesAfterDeployment = kuduService.listDir('/site/wwwroot/webapps/');
filesAfterDeployment.then((files) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Handle exceptions, wherever required.

vincent1173
vincent1173 approved these changes Mar 5, 2018
View reviewed changes
Tasks/AzureRmWebAppDeployment/operations/WarDeploymentUtilities.ts Outdated
// do a get call on the target web app.
await appServiceUtility.pingApplication();
var filesAfterDeployment = await kuduService.listDir('/site/wwwroot/webapps/');
tl.debug(tl.loc("FileExistingAfterDeployment") + JSON.stringify(filesAfterDeployment));
Copy link
Contributor

Choose a reason for hiding this comment

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

Localization not needed for debug statements.

Tasks/AzureRmWebAppDeployment/operations/WarDeploymentUtilities.ts Outdated

// do a get call on the target web app.
await appServiceUtility.pingApplication();
var filesAfterDeployment = await kuduService.listDir('/site/wwwroot/webapps/');
Copy link
Contributor

Choose a reason for hiding this comment

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

handle 404 case (it might return null)

vincent1173
vincent1173 reviewed Mar 5, 2018
View reviewed changes
Tasks/AzureRmWebAppDeployment/operations/WarDeploymentUtilities.ts Outdated

// Find if directory with same name as war file, existed before deployment
var directoryWithSameNameBeforeDeployment;
filesBeforeDeployment.some(item => {
Copy link
Contributor

@vincent1173 vincent1173 Mar 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

suggestion to initialize & iterate once. and save details of folder only.

@hiyadav hiyadav merged commit ce47b32 into master Mar 5, 2018
hiyadav added a commit that referenced this pull request Mar 6, 2018
@hiyadav
Users/hiyada/war deployment (#6579)
bb71703 
* resolving msdeploy for war files

* retrying three times for war files while calling the web app in between.

* resolving comments

* resolving comments

* removing duplicate file

* initializing variables only once insted of in retry.
hiyadav added a commit that referenced this pull request Mar 6, 2018
@hiyadav
Users/hiyada/war deployment port to m131 (#6609)
0458f8f 
* Users/hiyada/war deployment (#6579)

* resolving msdeploy for war files

* retrying three times for war files while calling the web app in between.

* resolving comments

* resolving comments

* removing duplicate file

* initializing variables only once insted of in retry.

* bumping up patch version  of AzureRmWebAppDeployment task
hiyadav added a commit that referenced this pull request Mar 6, 2018
@hiyadav
Users/hiyada/war deployment (#6579)
9800718 
* resolving msdeploy for war files

* retrying three times for war files while calling the web app in between.

* resolving comments

* resolving comments

* removing duplicate file

* initializing variables only once insted of in retry.
hiyadav added a commit that referenced this pull request Mar 6, 2018
@hiyadav
Users/hiyada/war deployment (#6579) (#6610)
edb6b8d 
* resolving msdeploy for war files

* retrying three times for war files while calling the web app in between.

* resolving comments

* resolving comments

* removing duplicate file

* initializing variables only once insted of in retry.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vincent1173 vincent1173 vincent1173 approved these changes

@kmkumaran kmkumaran Awaiting requested review from kmkumaran

@abhishea abhishea Awaiting requested review from abhishea

@Ajay-MS Ajay-MS Awaiting requested review from Ajay-MS

@sachinma sachinma Awaiting requested review from sachinma

Assignees

@kmkumaran kmkumaran

@sachinma sachinma

@vincent1173 vincent1173

@hiyadav hiyadav

Labels
Area: AzureAppService Label to monitor Azure App Service issues Area: Release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hiyadav @vincent1173 @kmkumaran @sachinma