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

[App Service] az staticwebapp functions link: Add new parameter --environment-name to support setting the environment name of static site #23894

Merged
merged 9 commits into from
Jan 5, 2023

Conversation

tobiasdiez
Copy link
Contributor

Related command

az staticwebapp functions link

Description

Currently, az staticwebapp functions link doesn't provide a way to link a certain environment/deployment slot to an existing function app. This functionality is however available through the web interface, and with this PR will be also available through the cli.

Testing Guide

Run az staticwebapp functions link as you normally would, but with an additional argument environment-name parameter. Afterwards, the function app should be linked only to that environment.

Note: I've not yet managed to test this myself, since I was running the code in codespaces and had troubles with the authentication.

History Notes

[Appservice] az staticwebapp functions link: Add environment-name parameter


This checklist is used to make sure that common guidelines for a pull request are followed.

@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Sep 14, 2022
@ghost
Copy link

ghost commented Sep 14, 2022

Thank you for your contribution tobiasdiez! We will review the pull request and get back to you soon.

@ghost ghost added the Auto-Assign Auto assign by bot label Sep 14, 2022
@ghost ghost requested review from wangzelin007 and yonzhan September 14, 2022 21:10
@ghost ghost assigned zhoxing-ms Sep 14, 2022
@ghost ghost added this to the Sep 2022 (2022-10-12) - For Ignite milestone Sep 14, 2022
@ghost ghost added the App Services az appservice label Sep 14, 2022
@yonzhan
Copy link
Collaborator

yonzhan commented Sep 14, 2022

Appservice

Copy link
Contributor

@StrawnSC StrawnSC left a comment

Choose a reason for hiding this comment

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

@tobiasdiez, did you try running the rewritten command locally? When I try it, fails with the exception TypeError: request() got an unexpected keyword argument 'environment_name'.
It looks like the SDK function begin_register_user_provided_function_app_with_static_site doesn't take an environment_name parameter. You would need to find a different SDK function that takes a parameter for the environment

@tobiasdiez
Copy link
Contributor Author

tobiasdiez commented Sep 16, 2022

Thanks for testing it. I have to admit I couldn't figure out how to test the change myself. How does one do it?

For the SDK function, the parameter environment_name is available, at least in the more recent versions:
https://github.com/Azure/azure-sdk-for-python/blob/7eeaca97c41bc18ec66f325eef5e6910efde6e12/sdk/appservice/azure-mgmt-web/azure/mgmt/web/v2022_03_01/aio/operations/_static_sites_operations.py#L1882

Edit: I overlooked that the function with the _build suffix is needed. Fixed now.

tobiasdiez added a commit to JabRef/JabRefOnline that referenced this pull request Sep 16, 2022
Fixes #1343. As outlined there, the azure cli currently doesn't provide
means to link a specific environment of a static web app to a function
app. This is why we use the python sdk directly. Maybe once
Azure/azure-cli#23894 is merged and released, we
can remove the script and use the cli.
@zhoxing-ms zhoxing-ms changed the title [Appservice] az staticwebapp functions link: Add environment-name parameter [App Service] az staticwebapp functions link: Add environment-name parameter Oct 8, 2022
@zhoxing-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@zhoxing-ms
Copy link
Contributor

@tobiasdiez Could you please resolve those CI issues?

@StrawnSC StrawnSC self-requested a review November 10, 2022 23:14
StrawnSC
StrawnSC previously approved these changes Nov 10, 2022
@zhoxing-ms
Copy link
Contributor

zhoxing-ms commented Nov 11, 2022

@mock.patch("azure.cli.command_modules.appservice.static_sites.show_app")

def test_functions_link(self, *args, **kwargs):
functionapp_name = "functionapp"
functionapp_resource_id = "/subscriptions/sub/resourceGroups/{}/providers/Microsoft.Web/sites/{}".format(
self.rg1, functionapp_name
)
link_user_function(self.mock_cmd, self.name1, self.rg1, functionapp_resource_id)
E TypeError: link_user_function() missing 1 required positional argument: 'environment_name'

@tobiasdiez Could you please resolve the CI issues?

@tobiasdiez
Copy link
Contributor Author

Oh, I missed that one. Should be fixed now.

@zhoxing-ms
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

StrawnSC
StrawnSC previously approved these changes Nov 30, 2022
Copy link
Contributor

@StrawnSC StrawnSC left a comment

Choose a reason for hiding this comment

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

Looks good to me, but we should commit @zhoxing-ms's suggestion and fix the CI

@zhoxing-ms
Copy link
Contributor

@tobiasdiez Any update? Please note that since we will launch the release of this sprint the day after tomorrow. If you cannot address all the comments tomorrow, we will have to postpone the release of this PR to the next sprint (02-07)

…s.py

Co-authored-by: Xing Zhou <Zhou.Xing@microsoft.com>
@tobiasdiez
Copy link
Contributor Author

Sorry for the late reaction and thanks @zhoxing-ms for the reminder. I've now committed your suggestion, so hopefully it should be ready to go now!

@zhoxing-ms
Copy link
Contributor

@StrawnSC Could you please help review this PR again? If there are no other questions here, please help approve it

@zhoxing-ms zhoxing-ms changed the title [App Service] az staticwebapp functions link: Add environment-name parameter [App Service] az staticwebapp functions link: Add new parameter --environment-name to support setting the environment name of static site Jan 5, 2023
@zhoxing-ms zhoxing-ms merged commit 171a932 into Azure:dev Jan 5, 2023
@tobiasdiez tobiasdiez deleted the codespace-a0b3 branch January 5, 2023 05:29
avgale pushed a commit to avgale/azure-cli that referenced this pull request Aug 24, 2023
…environment-name` to support setting the environment name of static site (Azure#23894)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App Services az appservice Auto-Assign Auto assign by bot customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants