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

NetworkCloud 2023-05-01 API scenarios tests created for Networkcloud … #6422

Merged

Conversation

priyamshet
Copy link
Member

@priyamshet priyamshet commented Jun 19, 2023

Related command

az networkcloud -h

Group
    az networkcloud : Manage Network Cloud resources.
        This command group is experimental and under development. Reference and support levels:
        https://aka.ms/CLI_refstatus
Subgroups:
    baremetalmachine     : Manage bare metal machine.
    cloudservicesnetwork : Manage cloud services network.
    cluster              : Manage cluster.
    clustermanager       : Manage cluster manager.
    defaultcninetwork    : Manage default CNI network.
    hybridakscluster     : Manage additional details of Hybrid AKS provisioned cluster.
    kubernetescluster    : Manage Kubernetes cluster.
    l2network            : Manage layer 2 (l2) network.
    l3network            : Manage layer 3 (l3) network.
    rack                 : Manage rack.
    racksku              : Manage rack SKU.
    storageappliance     : Manage storage appliance.
    trunkednetwork       : Manage trunked network.
    virtualmachine       : Manage virtual machine.
    volume               : Manage volume.

To search AI knowledge base for examples, use: az find "az networkcloud"

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 pull request is merged into main branch, a new pull request will be created to update src/index.json automatically.
You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify src/index.json.

NetworkCLoud AAZ PR : Azure/aaz#214

@azure-client-tools-bot-prd
Copy link

Hi @priyamshet,
Please write the description of changes which can be perceived by customers into HISTORY.rst.
If you want to release a new extension version, please update the version in setup.py as well.

@yonzhan
Copy link
Collaborator

yonzhan commented Jun 19, 2023

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

@priyamshet priyamshet force-pushed the priyashet/networkCloud2023-05-01apiupdate branch 5 times, most recently from 17bc276 to d90ec1f Compare June 19, 2023 20:57
@priyamshet priyamshet marked this pull request as ready for review June 19, 2023 21:19
@priyamshet
Copy link
Member Author

@kairu-ms Can we get request your review on this PR

Comment on lines 138 to 141
if list(args.ssh_dest_key_path):
ssh_keys += CustomSshOptions.get_ssh_keys_from_path(
list(args.ssh_dest_key_path)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use to_serialized_data() function, which will serialize an AAZValue to python build-in type value.

Suggested change
if list(args.ssh_dest_key_path):
ssh_keys += CustomSshOptions.get_ssh_keys_from_path(
list(args.ssh_dest_key_path)
)
if has_value(args.ssh_dest_key_path) and args.ssh_dest_key_path.to_serialized_data():
ssh_keys += CustomSshOptions.get_ssh_keys_from_path(
args.ssh_dest_key_path.to_serialized_data()
)

Copy link
Member Author

Choose a reason for hiding this comment

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

if we update to to_serialized_data() , we encounter an error that list object does not have a to_serialized_data() , are we missing anything here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you share the code raise this error?
The AAZList has to_serialized_data function, but the python build-in list doesn't have to_serialized_data function. The output of to_serialized_data for AAZList is a python list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Error This is what we see, please let us know if we are missing anything.

"""validate and add ssh key to the list"""

key_data = []
for key in values:
Copy link
Contributor

Choose a reason for hiding this comment

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

If the values comes from to_serialized_data(), the key is string type by default.

Copy link
Member Author

Choose a reason for hiding this comment

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

As the calling function could not be updated to use to_serialized_data() due to an error (details in previous comment), leaving this as this. please let us know if we missed anything.

@priyamshet priyamshet force-pushed the priyashet/networkCloud2023-05-01apiupdate branch 4 times, most recently from ee103db to e10adbe Compare June 21, 2023 22:44
@priyamshet priyamshet force-pushed the priyashet/networkCloud2023-05-01apiupdate branch from e10adbe to 5bb2b04 Compare June 22, 2023 18:42
@priyamshet priyamshet force-pushed the priyashet/networkCloud2023-05-01apiupdate branch from 5bb2b04 to 714dc2d Compare June 22, 2023 19:56
@priyamshet priyamshet requested a review from kairu-ms June 22, 2023 20:22
@priyamshet
Copy link
Member Author

@kairu-ms We have addressed most of your feedback, Can we request your re-review on this please.

@priyamshet priyamshet requested a review from kairu-ms June 26, 2023 22:24
@kairu-ms kairu-ms merged commit 3ef6230 into Azure:main Jun 27, 2023
@azclibot
Copy link
Collaborator

[Release] Update index.json for extension [ networkcloud ] : https://dev.azure.com/azclitools/internal/_build/results?buildId=67170&view=results

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.

4 participants