-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
[AppService] Fix - bug that prevented changing Container settings in Set-AzWebApp and Set-AzWebAppSlot #12107
Conversation
…` and `Set-AzWebAppSlot`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add test case for these changes
@VeryEarly - Added test cases. FYI @jvano @panchagnula |
@@ -539,14 +539,6 @@ public void UpdateWebAppConfiguration(string resourceGroupName, string location, | |||
|
|||
if (useSlot) | |||
{ | |||
if (siteConfig != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vinisoto - what was the scenario to move siteConfig
configuration update to behind the appSettings
configuration update. It is reported as regression issue in #14998 . please let me whether I can alter their sequence of updating the configuration or it should be like this for any of the other functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to make sure that the app settings below are updated before or at the same time as WindowsFxVersion (which is in SiteConfig). Otherwise, server side validation for WindowsFxVersion will fail since it will be trying to use "old" container settings.
public const string DockerRegistryServerUrl = "DOCKER_REGISTRY_SERVER_URL";
public const string DockerRegistryServerUserName = "DOCKER_REGISTRY_SERVER_USERNAME";
public const string DockerRegistryServerPassword = "DOCKER_REGISTRY_SERVER_PASSWORD";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vinisoto - For now I have reverted your changes. Because it has created a lot of mess in just few days. can you give me the #GitID for the same. I want to analyze it further to understand the issue and fix it if required.
@panchagnula - FYI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kotasudhakarreddy we should make sure to fix the issue Vini was working on as well with your changes so that we don't trade one regression for another. i.e this bug #13866
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
-Fix - bug that prevented changing Container settings in Set-AzWebApp and Set-AzWebAppSlot
-Fixed typos.
Checklist
CONTRIBUTING.md
ChangeLog.md
file(s) has been updated:ChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header -- no new version header should be added