Skip to content

Conversation

@leo79901
Copy link

@leo79901 leo79901 commented Dec 4, 2024

Description

Resolve the issur : Should not shutting down a server which is already shut.
Thanks for DaanHoogland which provide code.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI
  • test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

I am so sorry about this. I don't know much about Java, thus I have not tested...

How did you try to break this feature and the system with this change?

Resolve the issur : Should not shutting down a server which is already shut.
Thanks for DaanHoogland which provide code.
@boring-cyborg
Copy link

boring-cyborg bot commented Dec 4, 2024

Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
Here are some useful points:

@DaanHoogland
Copy link
Contributor

sorry to bug you with this, the linter says no. (just some trailing whitespace.)

@codecov
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 16.06%. Comparing base (27d2de1) to head (67e33f6).

Files with missing lines Patch % Lines
...va/org/apache/cloudstack/kvm/ha/KVMHAProvider.java 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main   #10039      +/-   ##
============================================
- Coverage     16.06%   16.06%   -0.01%     
- Complexity    12863    12864       +1     
============================================
  Files          5641     5641              
  Lines        493791   493794       +3     
  Branches      59858    59859       +1     
============================================
- Hits          79327    79326       -1     
- Misses       405683   405686       +3     
- Partials       8781     8782       +1     
Flag Coverage Δ
uitests 4.02% <ø> (ø)
unittests 16.90% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DaanHoogland
Copy link
Contributor

@slavkap @weizhouapache any sugestions for testing?

Also, I marked this for 4.21 @leo79901 , but it seems to me it can go onto 4.19. I think you'd like this fix sooner rather than later, right?

if you work with git regularly this is an easy thing to do, but as you say you are not a developer:

git rebase --onto 4.19 4ac4d9cf29abd83526093d162424da0419496297 3a6904094da9f84264c5a80659d334a561ff92b2

but these will change when you add work to it. Please let us know if you need help with this.

@leo79901
Copy link
Author

leo79901 commented Dec 5, 2024

@DaanHoogland
Thanks for your help!
Yes, I really want to use this in our environment which is 4.19.1.2. But i found that 'log' in 4.19 is 'LOG', and in 4.20 it is 'logger'. Do I need to edit the code again?

@DaanHoogland
Copy link
Contributor

@DaanHoogland Thanks for your help! Yes, I really want to use this in our environment which is 4.19.1.2. But i found that 'log' in 4.19 is 'LOG', and in 4.20 it is 'logger'. Do I need to edit the code again?

ai, yes. There has been a big logging change between 4.19 and 4.20 and you would indeed need to edit it. If you are planning to upgrade anyway and can life without till then you can change the commands to

git rebase --onto 4.20 4ac4d9cf29abd83526093d162424da0419496297 67e33f60146c923682d82c184c3fcd47cfd039c1

(changed to include the latest white space fix)

@leo79901
Copy link
Author

leo79901 commented Dec 6, 2024

Thanks really for your help.
I have run the command.
It's my great honor to contribute to the project. I will do more within my capacity in the future.

@DaanHoogland
Copy link
Contributor

Thanks really for your help. I have run the command. It's my great honor to contribute to the project. I will do more within my capacity in the future.

great @leo79901 , next step is to force push to your fork (git push --force)
and to edit this PR to point to the older branch (i can do that to show you)

In the future it will be better to create a branch locally and create PRs from such a branch. You are now having your changes on your local branch called main. This is not wroing but might be confusing.

@leo79901
Copy link
Author

leo79901 commented Dec 7, 2024

I'm terribly sorry for bothering you several times. I think I need to learn more about Git before submitting the PR. Could you please help me complete this operation this time?

@DaanHoogland
Copy link
Contributor

I'm terribly sorry for bothering you several times. I think I need to learn more about Git before submitting the PR. Could you please help me complete this operation this time?

no problem @leo79901 . I saw your other PR as well. I will copy your branch and create a PR from your code.

@DaanHoogland
Copy link
Contributor

@leo79901 I created #10059 for you, and will close this one. cc @weizhouapache

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants