removed code in comments #11145

DaanHoogland · 2025-07-04T13:34:10Z

Description

This PR fixes: #3777 using ai help a lot of code in comments was removed. By far not complete yet as “token expired”. Also I had to revert a lot of the suggestions as they where either completely invalid or not relevant to the premise “not-explanetary code in comment” , like diamond operators 🤯 or log rephrasing 😡 .

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

Screenshots (if appropriate):

How Has This Been Tested?

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

Copilot

Pull Request Overview

This PR cleans up commented-out code across multiple modules and updates various comment headers and Javadoc links.

Removed leftover debug and commented code in Java and Python files
Corrected license header comments in API commands
Updated Javadoc @see link formatting

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
utils/src/main/java/com/cloud/utils/xmlobject/XmlObjectParser.java	Removed commented-out debug `System.out.println`
utils/src/main/java/com/cloud/utils/xmlobject/XmlObject.java	Removed commented-out debug `System.out.println`
services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/template/UploadManagerImpl.java	Removed commented-out method call
services/secondary-storage/server/src/main/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResource.java	Removed commented-out path logic
services/console-proxy/rdpconsole/src/main/java/vncclient/vnc/Vnc33Hello.java	Removed commented debug properties
services/console-proxy/rdpconsole/src/main/java/rdpclient/rdp/ClientConfirmActivePDU.java	Updated Javadoc `@see` link formatting
services/console-proxy/rdpconsole/src/main/java/rdpclient/clip/ClipboardDataFormat.java	Removed commented-out clipboard formats
services/console-proxy/rdpconsole/src/main/java/rdpclient/RdpClient.java	Removed commented-out debug override
scripts/vm/hypervisor/xenserver/xenserver56fp1/NFSSR.py	Removed commented-out directory-creation logic
scripts/vm/hypervisor/xenserver/xcpserver/NFSSR.py	Removed commented-out directory-creation logic
scripts/vm/hypervisor/xenserver/xcposs/NFSSR.py	Removed commented-out directory-creation logic
scripts/vm/hypervisor/xenserver/perfmon.py	Removed commented-out XenAPI pool code
scripts/vm/hypervisor/ovm3/cloudstack.py	Removed commented-out OVS agent commands
plugins/storage/volume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolHelper.java	Removed commented-out custom logger setup
plugins/storage/volume/nexenta/src/main/java/org/apache/cloudstack/storage/datastore/driver/NexentaPrimaryDataStoreDriver.java	Removed commented-out DAO update call
plugins/network-elements/juniper-contrail/src/test/java/org/apache/cloudstack/network/contrail/management/ManagementServerMock.java	Simplified exception handling in test mock
plugins/network-elements/cisco-vnmc/src/main/java/com/cloud/network/cisco/CiscoVnmcConnectionImpl.java	Removed commented-out XML concatenation logic
engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/VolumeDataStoreVO.java	Removed commented-out constructor calls
engine/schema/src/main/java/com/cloud/dc/dao/ClusterVSMMapDaoImpl.java	Removed redundant `super()` call
engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/VolumeOrchestrator.java	Removed commented ISO-handling code
api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatePermissionsCmd.java	Corrected license header comment
api/src/main/java/org/apache/cloudstack/api/command/user/iso/ListIsoPermissionsCmd.java	Corrected license header comment
api/src/main/java/org/apache/cloudstack/api/command/admin/template/ListTemplatePermissionsCmdByAdmin.java	Corrected license header comment
api/src/main/java/org/apache/cloudstack/api/command/admin/iso/ListIsoPermissionsCmdByAdmin.java	Corrected license header comment

Comments suppressed due to low confidence (2)

services/console-proxy/rdpconsole/src/main/java/rdpclient/rdp/ClientConfirmActivePDU.java:30

[nitpick] The Javadoc link text is not descriptive; consider updating the anchor text to something like 'MSDN documentation for Client Confirm Active PDU' to improve clarity.

 * @see <a href="http://msdn.microsoft.com/en-us/library/cc240488.aspx">microsoft msdn explanation</a>

api/src/main/java/org/apache/cloudstack/api/command/user/template/ListTemplatePermissionsCmd.java:1

The license header seems incomplete and ends abruptly; please restore the full standard ASF header (e.g., 'under one or more contributor license agreements').

// Licensed to the Apache Software Foundation (ASF) under one

...il/src/test/java/org/apache/cloudstack/network/contrail/management/ManagementServerMock.java

codecov · 2025-07-04T13:42:44Z

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.29%. Comparing base (d7b7bd5) to head (1835662).
⚠️ Report is 116 commits behind head on main.

Files with missing lines	Patch %	Lines
.../cloud/network/CiscoNexusVSMDeviceManagerImpl.java	0.00%	1 Missing ⚠️
...er/src/main/java/com/cloud/test/IPRangeConfig.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main   #11145      +/-   ##
============================================
- Coverage     17.35%   17.29%   -0.07%     
- Complexity    15231    15249      +18     
============================================
  Files          5885     5889       +4     
  Lines        525611   526126     +515     
  Branches      64160    64235      +75     
============================================
- Hits          91224    90969     -255     
- Misses       424090   424857     +767     
- Partials      10297    10300       +3

Flag	Coverage Δ
uitests	`3.62% <ø> (-0.01%)`	⬇️
unittests	`18.32% <0.00%> (-0.07%)`	⬇️

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.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

harikrishna-patnala

minor comments added, overall LGTM

...c/test/java/org/apache/cloudstack/framework/codestyle/AsyncSampleEventDrivenStyleCaller.java

vishesh92 · 2025-09-18T10:36:26Z

api/src/main/java/org/apache/cloudstack/api/command/admin/iso/ListIsoPermissionsCmdByAdmin.java

@@ -1,4 +1,4 @@
-// Licensedname = "listIsoPermissions",  to the Apache Software Foundation (ASF) under one
+// Licensed to the Apache Software Foundation (ASF) under one


@DaanHoogland Just curious why the rat check didn't pick this up. Do we need any update in the github action?

maybe, it's not checking the license content

Or it found the needed string and the extra bit doesn’t matter…?

...il/src/test/java/org/apache/cloudstack/network/contrail/management/ManagementServerMock.java

vishesh92

clgtm

sureshanaparti

clgtm

DaanHoogland · 2025-09-19T09:11:31Z

@blueorangutan package

blueorangutan · 2025-09-19T09:12:04Z

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2025-09-19T11:46:52Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15101

DaanHoogland · 2025-09-19T13:31:13Z

@blueorangutan test matrix

blueorangutan · 2025-09-19T13:32:04Z

@DaanHoogland a [SL] Trillian-Jenkins matrix job (EL8 mgmt + EL8 KVM, Ubuntu22 mgmt + Ubuntu22 KVM, EL8 mgmt + VMware 7.0u3, EL9 mgmt + XCP-ng 8.2 ) has been kicked to run smoke tests

blueorangutan · 2025-09-20T06:07:43Z

[SF] Trillian test result (tid-14393)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 56322 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11145-t14393-kvm-ol8.zip
Smoke tests completed. 145 look OK, 2 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	1.16	test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster	`Failure`	36.46	test_kubernetes_clusters.py
test_01_isolate_network_FW_PF_default_routes_egress_true	`Failure`	107.68	test_routers_network_ops.py
test_02_isolate_network_FW_PF_default_routes_egress_false	`Failure`	198.11	test_routers_network_ops.py

blueorangutan · 2025-09-20T07:19:10Z

[SF] Trillian test result (tid-14394)
Environment: kvm-ubuntu22 (x2), zone: Advanced Networking with Mgmt server u22
Total time taken: 61330 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11145-t14394-kvm-ubuntu22.zip
Smoke tests completed. 146 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	1.18	test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster	`Failure`	39.38	test_kubernetes_clusters.py

blueorangutan · 2025-09-20T07:58:04Z

[SF] Trillian test result (tid-14395)
Environment: vmware-70u3 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 63244 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11145-t14395-vmware-70u3.zip
Smoke tests completed. 144 look OK, 3 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	1.18	test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster	`Failure`	33.19	test_kubernetes_clusters.py
test_01_ssl_offloading_isolated_network	`Failure`	797.37	test_ssl_offloading.py
test_02_ssl_offloading_project_vpc	`Failure`	775.56	test_ssl_offloading.py
test_01_vpn_usage	`Error`	1.10	test_usage.py

DaanHoogland · 2025-09-20T20:45:10Z

@blueorangutan test ol8 xcpng82

blueorangutan · 2025-09-20T20:46:03Z

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + xcpng82) has been kicked to run smoke tests

blueorangutan · 2025-09-21T19:29:13Z

[SF] Trillian test result (tid-14414)
Environment: xcpng82 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 79092 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr11145-t14414-xcpng82.zip
Smoke tests completed. 140 look OK, 7 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_01_events_resource	`Error`	209.60	test_events_resource.py
test_03_deploy_and_scale_kubernetes_cluster	`Failure`	1.18	test_kubernetes_clusters.py
test_04_autoscale_kubernetes_cluster	`Failure`	66.17	test_kubernetes_clusters.py
test_01_non_strict_host_anti_affinity	`Error`	193.63	test_nonstrict_affinity_group.py
test_02_non_strict_host_affinity	`Error`	55.58	test_nonstrict_affinity_group.py
test_01_primary_storage_iscsi	`Error`	0.88	test_primary_storage.py
test_01_volume_usage	`Failure`	94.91	test_usage.py
test_01_vpn_usage	`Error`	1.10	test_usage.py
test_11_destroy_vm_and_volumes	`Error`	19.93	test_vm_life_cycle.py
test_01_migrate_vm_strict_tags_success	`Error`	178.77	test_vm_strict_host_tags.py

harikrishna-patnala · 2025-09-22T05:51:25Z

@blueorangutan package

blueorangutan · 2025-09-22T05:52:04Z

@harikrishna-patnala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

harikrishna-patnala · 2025-09-22T06:13:46Z

we may move this PR to 4.22.1 as this has touched many files and no functional changes but it may cause trouble for existing PRs with conflicts. cc @DaanHoogland @weizhouapache

blueorangutan · 2025-09-22T07:11:33Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 15131

DaanHoogland · 2025-09-22T09:20:22Z

we may move this PR to 4.22.1 as this has touched many files and no functional changes but it may cause trouble for existing PRs with conflicts. cc @DaanHoogland @weizhouapache

makes sense but :( anyway ;|

DaanHoogland · 2025-10-17T10:29:30Z

@harikrishna-patnala , how about last minute merge?

DaanHoogland requested a review from Copilot July 4, 2025 13:34

boring-cyborg bot added component:api component:console-proxy component:networking component:orchestration component:primary-storage component:secondary-storage component:ssvm component:storage component:XenServer Python Warning... Python code Ahead! labels Jul 4, 2025

Copilot AI reviewed Jul 4, 2025

View reviewed changes

...il/src/test/java/org/apache/cloudstack/network/contrail/management/ManagementServerMock.java Show resolved Hide resolved

boring-cyborg bot added component:agent component:security-group labels Jul 4, 2025

DaanHoogland added type:cleanup and removed component:XenServer component:primary-storage component:secondary-storage component:console-proxy component:ssvm component:api component:security-group component:networking component:agent component:orchestration component:storage labels Jul 4, 2025

boring-cyborg bot added component:agent component:api component:networking labels Jul 4, 2025

weizhouapache modified the milestones: 4.21.0, 4.22.0 Aug 29, 2025

sureshanaparti removed this from Apache CloudStack 4.21.0 Aug 29, 2025

sureshanaparti added this to Apache CloudStack 4.22.0 Aug 29, 2025

sureshanaparti moved this to In Progress in Apache CloudStack 4.22.0 Aug 29, 2025

harikrishna-patnala approved these changes Sep 3, 2025

View reviewed changes

...c/test/java/org/apache/cloudstack/framework/codestyle/AsyncSampleEventDrivenStyleCaller.java Outdated Show resolved Hide resolved

...c/test/java/org/apache/cloudstack/framework/codestyle/AsyncSampleEventDrivenStyleCaller.java Outdated Show resolved Hide resolved

Apply suggestions from code review

0c59c11

vishesh92 reviewed Sep 18, 2025

View reviewed changes

...il/src/test/java/org/apache/cloudstack/network/contrail/management/ManagementServerMock.java Outdated Show resolved Hide resolved

vishesh92 approved these changes Sep 18, 2025

View reviewed changes

sureshanaparti approved these changes Sep 18, 2025

View reviewed changes

Apply suggestions from code review

1835662

apache deleted a comment from blueorangutan Sep 20, 2025

		@@ -1,4 +1,4 @@
		// Licensedname = "listIsoPermissions", to the Apache Software Foundation (ASF) under one
		// Licensed to the Apache Software Foundation (ASF) under one

removed code in comments #11145

Are you sure you want to change the base?

removed code in comments #11145

Uh oh!

Conversation

DaanHoogland commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

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

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

codecov bot commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

harikrishna-patnala left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vishesh92 Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

sureshanaparti Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vishesh92 left a comment

Choose a reason for hiding this comment

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Sep 19, 2025

Uh oh!

blueorangutan commented Sep 19, 2025

Uh oh!

blueorangutan commented Sep 19, 2025

Uh oh!

DaanHoogland commented Sep 19, 2025

Uh oh!

blueorangutan commented Sep 19, 2025

Uh oh!

blueorangutan commented Sep 20, 2025

Uh oh!

blueorangutan commented Sep 20, 2025

Uh oh!

blueorangutan commented Sep 20, 2025

Uh oh!

DaanHoogland commented Sep 20, 2025

Uh oh!

blueorangutan commented Sep 20, 2025

Uh oh!

blueorangutan commented Sep 21, 2025

Uh oh!

harikrishna-patnala commented Sep 22, 2025

Uh oh!

blueorangutan commented Sep 22, 2025

Uh oh!

harikrishna-patnala commented Sep 22, 2025

Uh oh!

blueorangutan commented Sep 22, 2025

Uh oh!

DaanHoogland commented Sep 22, 2025

Uh oh!

DaanHoogland commented Oct 17, 2025

DaanHoogland commented Jul 4, 2025 •

edited

Loading

codecov bot commented Jul 4, 2025 •

edited

Loading