Skip to content

spring-cloud: add location to Apps operation#1458

Merged
fengzhou-msft merged 6 commits intoAzure:masterfrom
xgugeng:hkn/spring-app-location
Apr 2, 2020
Merged

spring-cloud: add location to Apps operation#1458
fengzhou-msft merged 6 commits intoAzure:masterfrom
xgugeng:hkn/spring-app-location

Conversation

@xgugeng
Copy link
Copy Markdown

@xgugeng xgugeng commented Mar 30, 2020


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

General Guidelines

  • Have you run azdev style <YOUR_EXT> locally? (pip install azdev required)
  • Have you run python scripts/ci/test_index.py -q locally?

For new extensions:

About Extension Publish

There is a pipeline to automatically build, upload and publish extension wheels.
Once your PR is merged into master branch, a new PR will be created to update src/index.json automatically.
The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify src/index.json.

@azuresdkci
Copy link
Copy Markdown

If this PR is for a new extension or change to an existing extension, use the following to try out the changes in this PR:

docker run -it microsoft/azure-cli:latest
export EXT=<NAME>
pip install --upgrade --target ~/.azure/cliextensions/$EXT "git+https://github.com/hellokangning/azure-cli-extensions.git@hkn/spring-app-location#subdirectory=src/$EXT&egg=$EXT"

@yonzhan yonzhan requested a review from qwordy March 30, 2020 02:15
@yonzhan yonzhan added this to the S168 milestone Mar 30, 2020
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented Mar 30, 2020

add to S168

@yungezz yungezz assigned fengzhou-msft and unassigned yungezz Mar 30, 2020
logger.warning("[1/2] updating app '{}'".format(name))
poller = client.apps.update(
resource_group, service, name, properties)
resource_group, service, name, properties, location)
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.

why location required in each rest call. suppose it should not in update parameter in swagger

Copy link
Copy Markdown
Author

@xgugeng xgugeng Mar 31, 2020

Choose a reason for hiding this comment

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

We are migrating Microsoft.AppPlatform/Spring/Apps from proxy resource to tracked resource. And location is a must for tracked resource. I've also updated the swagger change in Azure/azure-rest-api-specs#8851

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.

If this location is always the same as the location of the resource_group, why not fetch it from the resource group on the service side? Or location could be different in the future?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not the same with the location of resource_group, but with the one of Microsoft.AppPlatform/Spring. We cannot do it on service side, since it's ARM consuming this property, before our RP.

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.

I see. Thanks for the explanation.

@fengzhou-msft fengzhou-msft merged commit 93ac7c0 into Azure:master Apr 2, 2020
ManuInNZ pushed a commit to ManuInNZ/azure-cli-extensions that referenced this pull request Apr 11, 2020
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.

6 participants