-
Notifications
You must be signed in to change notification settings - Fork 1.5k
{AKS} Added namespace changes in aks cli #4767
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
base: main
Are you sure you want to change the base?
Conversation
|
aks |
|
@FumingZhang PTAL |
| resource_name: str, | ||
| server_fqdn: Optional[str] = None, | ||
| format: Optional[Union[str, "_models.Format"]] = None, | ||
| namespace_name = 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.
It's not a good idea to modify the SDK manually. If this feature is written in swagger, the automatically generated SDK would include the changes. Otherwise, these changes will be erased in the next SDK update (expected to update to 04-02-preview in just a few days).
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.
I removed the changes here and added the namespace sdk generated from the namespace swagger. The namespace swagger is separate from the AKS swagger. Regenerating the AKS sdk will not remove the namespace sdk, will it?
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.
That's OK. If the namespace service has python SDK officially released, may use it without vendor the code.
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.
cc @norshtein for awareness of modifying the get-credentials command.
…kK123/azure-cli-extensions into kkanukollu/aks-namespaces
…kK123/azure-cli-extensions into kkanukollu/aks-namespaces
FumingZhang
left a comment
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
src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py
Outdated
Show resolved
Hide resolved
FumingZhang
left a comment
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
|
Hey @KarthikK123, could you please clean up the unrelated changes in your branch? |
This checklist is used to make sure that common guidelines for a pull request are followed.
General Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally?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.jsonautomatically.The precondition is to put your code inside this repo and upgrade the version in the PR but do not modify
src/index.json.