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
Fixes bug with adding hostname to web app. #1985
Conversation
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.
LGTM--but please fix the parameter casing.
@@ -173,28 +173,27 @@ def show_container_settings(resource_group_name, name, slot=None): | |||
def _filter_for_container_settings(settings): | |||
return {x: settings[x] for x in settings if x in CONTAINER_APPSETTING_NAMES} | |||
|
|||
def add_hostname(resource_group_name, webapp, name, slot=None): | |||
def add_hostname(resource_group_name, webappName, name, slot=None): |
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.
This should be Python casing (i.e. webapp_name).
webapp = client.sites.get_site(resource_group_name, webapp) | ||
binding = HostNameBinding(webapp.location, host_name_binding_name=name, site_name=webapp) | ||
webapp = client.sites.get_site(resource_group_name, webappName) | ||
binding = HostNameBinding(webapp.location, host_name_binding_name=name, site_name=webapp.name) |
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.
So the problem was the parameter was redefined (webapp as a string in the method signature redefined to webapp the object). You could also use 'webapp_name' here too.
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.
Good, good, will do. I know the other methods were unaffected, but I wanted to keep them consistent. I will update today.
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.
Looks good.
Thanks all! |
Fixes #1984.