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

Users/suaggar/dynamic extension list #7071

Merged
merged 8 commits into from
May 2, 2018

Conversation

SumiranAgg
Copy link
Contributor

@SumiranAgg SumiranAgg commented Apr 25, 2018

Dynamic install extensions drop down

Issue to remove escape character in extension title:
toby-meyer/AzSecurityPackRedirectAndHeadersMed#4

@SumiranAgg SumiranAgg requested a review from vincent1173 April 25, 2018 10:43
@vincent1173 vincent1173 requested a review from kmkumaran April 25, 2018 11:01
@vincent1173 vincent1173 added Area: Release Area: AzureAppService Label to monitor Azure App Service issues labels Apr 25, 2018

for(var extensionID of extensionList) {
var siteExtensionDetails = null;

// Python extensions are moved to Nuget and the extensions IDs are changed. The belo check ensures that old extensions are mapped to new extension ID.
if(!allSiteExtensionMap[extensionID]){
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: space before {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@@ -129,6 +129,26 @@ export class KuduTests {
}
}

public static async getAllSiteExtensions() {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 for L0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :)

var extensionLocalPaths: string = "";
for(var siteExtension of siteExtensions) {
siteExtensionMap[siteExtension.id] = siteExtension;
}
for(var siteExtension of allSiteExtensions) {
allSiteExtensionMap[siteExtension.id] = siteExtension;
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Can we include the title too in the map?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

@@ -141,6 +141,7 @@
"loc.messages.FailedToStartContinuousWebJob": "Failed to start continuous WebJob '%s'. Error: %s",
"loc.messages.FailedToStopContinuousWebJob": "Failed to stop continuous WebJob '%s'. Error: %s",
"loc.messages.FailedToGetSiteExtensions": "Failed to get site extensions. Error: %s",
"loc.messages.FailedToGetAllSiteExtensions": "Failed to get all site extensions. Error: %s",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Failed to get extension feed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

incorporated


for(var extensionID of extensionList) {
var siteExtensionDetails = null;

// Python extensions are moved to Nuget and the extensions IDs are changed. The belo check ensures that old extensions are mapped to new extension ID.
if(allSiteExtensionMap[extensionID] && allSiteExtensionMap[extensionID].title == extensionID) {
Copy link
Contributor

@vincent1173 vincent1173 Apr 26, 2018

Choose a reason for hiding this comment

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

Can we simplify the condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we are updating extensionID only when its value is matching title of some siteextension.

@vincent1173
Copy link
Contributor

#7036

@SumiranAgg SumiranAgg merged commit f0ad314 into master May 2, 2018
@SumiranAgg SumiranAgg deleted the users/suaggar/dynamicExtensionList branch May 4, 2018 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: AzureAppService Label to monitor Azure App Service issues Area: Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants