webapp: Updating AppService to use the new Python SDK version #8886
Conversation
panchagnula
force-pushed
the
sisirap-newSDKWebappApps
branch
from
2069115 to
edea95b
Compare
There was a problem hiding this comment.
This should use the CLI's built-in deprecation mechanisms.
There was a problem hiding this comment.
@tjprescott can you point me to an example of this when deprecating a property - the examples i saw were all where the command was being deprecated. thank you!
There was a problem hiding this comment.
tjprescott
left a comment
tjprescott
left a comment
There was a problem hiding this comment.
Please use the built-in deprecation mechanism in the CLI.
|
Also @tjprescott i see this error in the build -> "azure-cli-appservice 0.2.17 has requirement azure-mgmt-web==0.41.0, but you'll have azure-mgmt-web 0.40.0 which is incompatible." though i updated the setup.py as a part of my PR. I don't understand this error? |
|
@panchagnula, the reason is the botservice also references the azure-mgmt-web, so do update their command module as well |
panchagnula
force-pushed
the
sisirap-newSDKWebappApps
branch
from
57e26db to
9ecdea9
Compare
panchagnula
force-pushed
the
sisirap-newSDKWebappApps
branch
from
a9f0ffa to
c8c2c15
Compare
…ding a bug to fix & add this test back
panchagnula
force-pushed
the
sisirap-newSDKWebappApps
branch
from
c8c2c15 to
3e6d77d
Compare
|
@yugangw-msft look good to you? |
yugangw-msft
left a comment
yugangw-msft
left a comment
There was a problem hiding this comment.
One question to clarify, after that this PR can be merged
| @@ -1186,10 +1186,6 @@ def test_webapp_assign_system_identity(self, resource_group): | |||
| self.cmd('webapp identity show -g {} -n {}'.format(resource_group, webapp_name), checks=[ | |||
| self.check('principalId', result['principalId']) | |||
| ]) | |||
| self.cmd('role assignment list -g {} --assignee {}'.format(resource_group, result['principalId']), checks=[ | |||
There was a problem hiding this comment.
Two reasons
- the check did not seem to add any value to this specific test for MSI assignment for webapps
- This call for role assignment list is returning an empty object -> even without my changes , ex. if i run this on PROD CLI
I was trying not to take dependency on non webapp calls here - also honestly i don't understand the role assignment call & what the expected value should be here. If you believe this is valuable for the test i can create a bug to investigate & add this back, but i don't think this is blocking this PR. what do you think?
There was a problem hiding this comment.
We have a big regression here, if role assignment list returns nothing. Note, 'webapp identity assign is supposed to create such a role assignment for you. Please do cross check here. W/o role assignment, the identity is pretty much useless.
There was a problem hiding this comment.
Cross checked & if this is a regression i will start an investigation with the backend folks on this, but this is the behavior i am seeing on PROD as well. Can we keep this investigation orthogonal to the PR?
There was a problem hiding this comment.
// CC: @praries880 who is working on the Java SDK support on this. Please post update here. If turns on a service end issue, please share with us the ICM ticket
4 participants
This checklist is used to make sure that common guidelines for a pull request are followed.
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.