Skip to content

appservice: generic update for web/functionapp#6295

Merged
yugangw-msft merged 5 commits into
Azure:devfrom
yugangw-msft:asu
May 8, 2018
Merged

appservice: generic update for web/functionapp#6295
yugangw-msft merged 5 commits into
Azure:devfrom
yugangw-msft:asu

Conversation

@yugangw-msft
Copy link
Copy Markdown
Contributor

fIx #5547 fix #5793

  • The PR has modified HISTORY.rst describing any customer-facing, functional changes. Note that this does not include changes only to help content. (see Modifying change log).

  • I adhere to the Command Guidelines.

@promptws
Copy link
Copy Markdown

promptws commented May 5, 2018

View a preview at https://prompt.ws/r/Azure/azure-cli/6295
This is an experimental preview for @microsoft users.

with self.argument_context('webapp update') as c:
c.argument('client_affinity_enabled', help="Enables sending session affinity cookies.", arg_type=get_three_state_flag(return_label=True))
c.argument('https_only', help="Redirect all traffic made to an app using HTTP to HTTPS.", arg_type=get_three_state_flag(return_label=True))
c.argument('force_dns_registration', help="If true, web app hostname is force registered with DNS", arg_type=get_three_state_flag(return_label=True))
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

those are command arguments which are already exposed through the update. Because I updated the wiring up mechanism for slot support which involves less auto-command support, hence I have to manually author the command argument here.

@tjprescott
Copy link
Copy Markdown
Member

test_functionapp_e2e is failing on CI.

Copy link
Copy Markdown
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

We need to keep the processing order consistent with other generic update commands. Let me know if you run into trouble splitting the setter out.

g.custom_command('list-consumption-locations', 'list_consumption_locations')
g.custom_command('identity assign', 'assign_identity')
g.custom_command('identity show', 'show_identity')
g.generic_update_command('update', getter_name='get_app', setter_name='update_functionapp', command_type=appservice_custom)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To preserve the generic update "contract", you need to separate the setter (the PUT) from processing the convenience arguments. Otherwise, the convenience arguments will trump the generic arguments in the event of a conflict, which is opposite of other updates.

So you'll need to specify the custom_func_name here to expose the convenience arguments.

if client_affinity_enabled is not None:
instance.client_affinity_enabled = client_affinity_enabled == 'true'
if https_only is not None:
instance.https_only = https_only == 'true'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All of these instance.<propery> entries will remain part of the custom func. It looks like that function will end here and return instance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think I have done exactly you suggested, please double check.

instance.https_only = https_only == 'true'
return instance

client = web_client_factory(cmd.cli_ctx)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Then it looks like the custom setter will begin here and you can call the appropriate API by whether slot is empty or has a value. In this case, I don't think you'll even need a hidden tracking argument.

Copy link
Copy Markdown
Member

@tjprescott tjprescott left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @yugangw-msft!

@yugangw-msft yugangw-msft merged commit fa3cab6 into Azure:dev May 8, 2018
@yugangw-msft yugangw-msft deleted the asu branch May 8, 2018 20:09
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