Skip to content

[HDInsight] Add two commands to support hdinsight node reboot feature #14005

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.

Sign up for GitHub

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

Jump to bottom
Merged
Juliehzl merged 12 commits into from
Jul 2, 2020
Merged

[HDInsight] Add two commands to support hdinsight node reboot feature #14005

Juliehzl merged 12 commits into from
Jul 2, 2020

Conversation

@aim-for-better
Copy link
Member

@aim-for-better aim-for-better commented Jun 18, 2020 •
edited by Juliehzl
Loading

Description
Add two commands:

  • az hdinsight host list to support list the hosts of the HDInsight cluster
  • az hdinsight host restart to support restart specific hosts of the HDInsight cluster

Testing Guide

History Notes

az hdinsight host list to support list the hosts of the HDInsight cluster
az hdinsight host restart to support restart specific hosts of the HDInsight cluster

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

@aim-for-better aim-for-better requested a review from idear1203 June 18, 2020 03:45
@aim-for-better aim-for-better changed the title [HDInsight] BREAKING CHANGE Add for hdinsight node reboot feature [HDInsight] Add two commands to support hdinsight node reboot feature Jun 18, 2020
@yonzhan yonzhan requested a review from jsntcy June 18, 2020 05:49
@yonzhan yonzhan added this to the S172 milestone Jun 18, 2020
@yonzhan
Copy link
Collaborator

yonzhan commented Jun 18, 2020

HDInsight

mmyyrroonn
mmyyrroonn reviewed Jun 18, 2020
View reviewed changes
Copy link
Contributor

@mmyyrroonn mmyyrroonn left a comment

Choose a reason for hiding this comment

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

Could you add tests for these two commands?

@aim-for-better
Copy link
Member Author

aim-for-better commented Jun 29, 2020

Could you add tests for these two commands?

Hi @myronfanqiu I have added the related tests. Could you please review again?

mmyyrroonn
mmyyrroonn reviewed Jun 30, 2020
View reviewed changes
src/azure-cli/azure/cli/command_modules/hdinsight/_params.py Outdated
Copy link
Contributor

@mmyyrroonn mmyyrroonn Jun 30, 2020

Choose a reason for hiding this comment

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

The name of this argument is a little bit confusing for me. Should we add options_list for it?

Copy link
Member Author

@aim-for-better aim-for-better Jun 30, 2020

Choose a reason for hiding this comment

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

The name of this argument is a little bit confusing for me. Should we add options_list for it?

Updated

@mmyyrroonn
Copy link
Contributor

mmyyrroonn commented Jun 30, 2020

@haroldrandom @Juliehzl Please review this PR as the code onwer.

@aim-for-better aim-for-better requested review from mmyyrroonn and removed request for jsntcy June 30, 2020 03:26
Juliehzl
Juliehzl reviewed Jun 30, 2020
View reviewed changes
src/azure-cli/azure/cli/command_modules/hdinsight/commands.py Outdated
Copy link
Contributor

@Juliehzl Juliehzl Jun 30, 2020

Choose a reason for hiding this comment

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

If support no wait, could we add wait command to wait till condition matched?

Copy link
Member Author

@aim-for-better aim-for-better Jul 1, 2020

Choose a reason for hiding this comment

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

If support no wait, could we add wait command to wait till condition matched?

Hi @Juliehzl Thanks for reminder. I have added the wait command.

Copy link
Member Author

@aim-for-better aim-for-better Jul 1, 2020

Choose a reason for hiding this comment

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

If support no wait, could we add wait command to wait till condition matched?

Hi @Juliehzl Thanks for reminder. I have added the wait command.

Hi @Juliehzl As we talked I removed support_no_wait, Thank you very much!

@aim-for-better aim-for-better requested a review from Juliehzl July 1, 2020 04:10
@aim-for-better aim-for-better force-pushed the HdiSupportNodeReboot branch from b5315c1 to 6327623 Compare July 1, 2020 07:09
@aim-for-better aim-for-better force-pushed the HdiSupportNodeReboot branch from 6327623 to 3dbfb7e Compare July 1, 2020 08:11
@Juliehzl Juliehzl merged commit efbcd5d into Azure:dev Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Juliehzl Juliehzl Juliehzl approved these changes

@haroldrandom haroldrandom Awaiting requested review from haroldrandom

@idear1203 idear1203 Awaiting requested review from idear1203

@mmyyrroonn mmyyrroonn Awaiting requested review from mmyyrroonn

Assignees

@mmyyrroonn mmyyrroonn

Labels

None yet

Projects

None yet

Milestone

S172

Development

Successfully merging this pull request may close these issues.

6 participants

@aim-for-better @yonzhan @mmyyrroonn @Juliehzl @dkmiller @azclibot