removed unused code in snapshotDao #2659

DaanHoogland · 2018-05-22T08:39:51Z

Description

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)

GitHub Issue/PRs

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

I have read the CONTRIBUTING document.
My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
Testing
I have added tests to cover my changes.
All relevant new and existing integration tests have passed.
A full integration testsuite with all test that can run on my environment has passed.

DaanHoogland · 2018-05-22T08:40:17Z

@blueorangutan package

blueorangutan · 2018-05-22T08:40:53Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2018-05-22T09:07:52Z

Packaging result: ✔centos6 ✔centos7 ✖debian. JID-2073

rafaelweingartner · 2018-05-22T10:07:55Z

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java

-
-        TransactionLegacy txn = TransactionLegacy.currentTxn();
-        PreparedStatement pstmt = null;
-        String sql = GET_SECHOST_ID;


The GET_SECHOST_ID variable is also only used here. So, what about if you delete it as well?

rafaelweingartner · 2018-05-22T10:08:24Z

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java

-    public long updateSnapshotVersion(long volumeId, String from, String to) {
-        TransactionLegacy txn = TransactionLegacy.currentTxn();
-        PreparedStatement pstmt = null;
-        String sql = UPDATE_SNAPSHOT_VERSION;


The same happens here.

rafaelweingartner · 2018-05-22T10:08:57Z

engine/schema/src/main/java/com/cloud/storage/dao/SnapshotDaoImpl.java

-    public long updateSnapshotSecHost(long dcId, long secHostId) {
-        TransactionLegacy txn = TransactionLegacy.currentTxn();
-        PreparedStatement pstmt = null;
-        String sql = UPDATE_SECHOST_ID;


thanks, will do

DaanHoogland · 2018-05-22T11:01:38Z

@blueorangutan package

blueorangutan · 2018-05-22T11:01:52Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2018-05-22T11:32:32Z

Packaging result: ✔centos6 ✔centos7 ✖debian. JID-2076

GabrielBrascher

Code LGTM.

sureshanaparti · 2018-05-22T12:23:47Z

@DaanHoogland Not related to dao. I observed that the Event "BackedupToSecondary" in Snapshot class is never used. Please take a look and if not required, clean that. Thanks.

borisstoyanov · 2018-05-23T08:16:21Z

@blueorangutan test

blueorangutan · 2018-05-23T08:16:53Z

@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2018-05-24T05:32:14Z

Trillian test result (tid-2699)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 71854 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2659-t2699-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermitten failure detected: /marvin/tests/smoke/test_password_server.py
Intermitten failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_router_dhcphosts.py
Intermitten failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 55 look OK, 12 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
ContextSuite context=TestDeployVirtioSCSIVM>:setup	`Error`	0.00	test_deploy_virtio_scsi_vm.py
test_02_internallb_roundrobin_1RVPC_3VM_HTTP_port80	`Failure`	414.38	test_internal_lb.py
test_01_add_primary_storage_disabled_host	`Error`	1.11	test_primary_storage.py
test_01_primary_storage_nfs	`Error`	0.11	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.20	test_primary_storage.py
test_02_vpc_privategw_static_routes	`Failure`	381.37	test_privategw_acl.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	394.13	test_privategw_acl.py
test_04_rvpc_privategw_static_routes	`Failure`	724.74	test_privategw_acl.py
test_router_dhcphosts	`Failure`	202.90	test_router_dhcphosts.py
ContextSuite context=TestRouterDHCPHosts>:teardown	`Error`	219.44	test_router_dhcphosts.py
test_01_isolate_network_FW_PF_default_routes_egress_true	`Failure`	373.89	test_routers_network_ops.py
test_02_isolate_network_FW_PF_default_routes_egress_false	`Failure`	331.97	test_routers_network_ops.py
test_01_RVR_Network_FW_PF_SSH_default_routes_egress_true	`Failure`	655.90	test_routers_network_ops.py
test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false	`Failure`	896.49	test_routers_network_ops.py
test_02_list_snapshots_with_removed_data_store	`Error`	1.15	test_snapshots.py
test_01_secured_vm_migration	`Error`	46.68	test_vm_life_cycle.py
test_02_not_secured_vm_migration	`Error`	44.63	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	`Error`	47.70	test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migration	`Error`	45.67	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	324.53	test_vm_life_cycle.py
test_04_rvpc_network_garbage_collector_nics	`Failure`	528.02	test_vpc_redundant.py
test_01_redundant_vpc_site2site_vpn	`Failure`	971.18	test_vpc_vpn.py
test_01_vpc_site2site_vpn_multiple_options	`Error`	516.94	test_vpc_vpn.py
test_01_vpc_site2site_vpn	`Failure`	441.32	test_vpc_vpn.py
test_01_cancel_host_maintenace_with_no_migration_jobs	`Failure`	0.13	test_host_maintenance.py
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	2.36	test_host_maintenance.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	2.62	test_hostha_kvm.py

DaanHoogland · 2018-05-24T06:56:23Z

A lot of unrelated errors here. I had a look at the snapshot testsuite and the exception has to do with creating a storage pool and not with the snapshotting itself. So also this failure is not related to this change.

nitin-maharana

Code LGTM.

rafaelweingartner

@DaanHoogland can you rebase with the new master's head? Then, we can see if the errors disapear.

borisstoyanov

@DaanHoogland there's lots of errors, can you rebase so we can rerun to make sure they are not related

DaanHoogland · 2018-06-26T07:55:41Z

@blueorangutan package

blueorangutan · 2018-06-26T07:56:44Z

@DaanHoogland a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2018-06-26T08:29:48Z

Packaging result: ✔centos6 ✔centos7 ✔debian. JID-2153

borisstoyanov · 2018-06-26T10:24:32Z

@blueorangutan test

blueorangutan · 2018-06-26T10:24:44Z

@borisstoyanov a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

blueorangutan · 2018-06-26T17:04:06Z

Trillian test result (tid-2826)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 22408 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr2659-t2826-kvm-centos7.zip
Intermitten failure detected: /marvin/tests/smoke/test_certauthority_root.py
Intermitten failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermitten failure detected: /marvin/tests/smoke/test_primary_storage.py
Intermitten failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermitten failure detected: /marvin/tests/smoke/test_snapshots.py
Intermitten failure detected: /marvin/tests/smoke/test_ssvm.py
Intermitten failure detected: /marvin/tests/smoke/test_vm_life_cycle.py
Intermitten failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermitten failure detected: /marvin/tests/smoke/test_host_maintenance.py
Intermitten failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 59 look OK, 8 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_provision_certificate	`Error`	5.16	test_certauthority_root.py
ContextSuite context=TestDeployVirtioSCSIVM>:setup	`Error`	0.00	test_deploy_virtio_scsi_vm.py
test_01_add_primary_storage_disabled_host	`Error`	0.62	test_primary_storage.py
test_01_primary_storage_nfs	`Error`	0.11	test_primary_storage.py
ContextSuite context=TestStorageTags>:setup	`Error`	0.20	test_primary_storage.py
test_03_vpc_privategw_restart_vpc_cleanup	`Failure`	1105.58	test_privategw_acl.py
test_02_list_snapshots_with_removed_data_store	`Error`	1.14	test_snapshots.py
ContextSuite context=TestDeployVM>:setup	`Error`	0.00	test_vm_life_cycle.py
test_01_secure_vm_migration	`Error`	0.06	test_vm_life_cycle.py
test_02_unsecure_vm_migration	`Error`	1.09	test_vm_life_cycle.py
test_03_secured_to_nonsecured_vm_migration	`Error`	1.08	test_vm_life_cycle.py
test_04_nonsecured_to_secured_vm_migration	`Error`	1.09	test_vm_life_cycle.py
test_08_migrate_vm	`Error`	15.69	test_vm_life_cycle.py
test_01_cancel_host_maintenace_with_no_migration_jobs	`Failure`	6.49	test_host_maintenance.py
test_02_cancel_host_maintenace_with_migration_jobs	`Error`	2.64	test_host_maintenance.py
test_hostha_enable_ha_when_host_in_maintenance	`Error`	1.45	test_hostha_kvm.py

borisstoyanov

LGTM, tests seems to be repeating

* removed unused code * remove sql strings

DaanHoogland changed the title ~~removed unused code~~ removed unused code in snapshotDao May 22, 2018

rafaelweingartner reviewed May 22, 2018

View reviewed changes

GabrielBrascher added this to the 4.12.0.0 milestone May 22, 2018

GabrielBrascher approved these changes May 22, 2018

View reviewed changes

nitin-maharana approved these changes May 26, 2018

View reviewed changes

rafaelweingartner approved these changes Jun 20, 2018

View reviewed changes

borisstoyanov reviewed Jun 25, 2018

View reviewed changes

DaanHoogland added 2 commits June 26, 2018 09:54

removed unused code

cda228c

remove sql strings

9466142

DaanHoogland force-pushed the snapshotCleanup branch from 68983c2 to 9466142 Compare June 26, 2018 07:55

borisstoyanov approved these changes Jun 27, 2018

View reviewed changes

DaanHoogland merged commit d0d3895 into apache:master Jul 3, 2018

ernjvr pushed a commit to shapeblue/cloudstack that referenced this pull request Jul 3, 2018

removed unused code in snapshotDao (apache#2659)

0c0fb91

* removed unused code * remove sql strings

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

removed unused code in snapshotDao #2659

removed unused code in snapshotDao #2659

DaanHoogland commented May 22, 2018

DaanHoogland commented May 22, 2018

blueorangutan commented May 22, 2018

blueorangutan commented May 22, 2018

rafaelweingartner May 22, 2018

DaanHoogland May 22, 2018

rafaelweingartner May 22, 2018

DaanHoogland May 22, 2018

rafaelweingartner May 22, 2018

DaanHoogland May 22, 2018

DaanHoogland May 22, 2018

DaanHoogland commented May 22, 2018

blueorangutan commented May 22, 2018

blueorangutan commented May 22, 2018

GabrielBrascher left a comment

sureshanaparti commented May 22, 2018

borisstoyanov commented May 23, 2018

blueorangutan commented May 23, 2018

blueorangutan commented May 24, 2018

DaanHoogland commented May 24, 2018

nitin-maharana left a comment

rafaelweingartner left a comment •

edited

borisstoyanov left a comment

DaanHoogland commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

borisstoyanov commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

borisstoyanov left a comment

removed unused code in snapshotDao #2659

removed unused code in snapshotDao #2659

Conversation

DaanHoogland commented May 22, 2018

Description

Types of changes

GitHub Issue/PRs

Screenshots (if appropriate):

How Has This Been Tested?

Checklist:

DaanHoogland commented May 22, 2018

blueorangutan commented May 22, 2018

blueorangutan commented May 22, 2018

rafaelweingartner May 22, 2018

Choose a reason for hiding this comment

DaanHoogland May 22, 2018

Choose a reason for hiding this comment

rafaelweingartner May 22, 2018

Choose a reason for hiding this comment

DaanHoogland May 22, 2018

Choose a reason for hiding this comment

rafaelweingartner May 22, 2018

Choose a reason for hiding this comment

DaanHoogland May 22, 2018

Choose a reason for hiding this comment

DaanHoogland May 22, 2018

Choose a reason for hiding this comment

DaanHoogland commented May 22, 2018

blueorangutan commented May 22, 2018

blueorangutan commented May 22, 2018

GabrielBrascher left a comment

Choose a reason for hiding this comment

sureshanaparti commented May 22, 2018

borisstoyanov commented May 23, 2018

blueorangutan commented May 23, 2018

blueorangutan commented May 24, 2018

DaanHoogland commented May 24, 2018

nitin-maharana left a comment

Choose a reason for hiding this comment

rafaelweingartner left a comment • edited

Choose a reason for hiding this comment

borisstoyanov left a comment

Choose a reason for hiding this comment

DaanHoogland commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

borisstoyanov commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

blueorangutan commented Jun 26, 2018

borisstoyanov left a comment

Choose a reason for hiding this comment

rafaelweingartner left a comment •

edited