Move views into separate files for easier modification #8647

DaanHoogland · 2024-02-13T14:30:51Z

Description

we moved to a new way of maintaining views (see #7417)
not all views were added in that PR so this pr is intended to do so

EXPECTED RESULTS

$ ls -1 engine/schema/src/main/resources/META-INF/db/views/
cloud.account_netstats_view.sql
cloud.account_view.sql
cloud.account_vmstats_view.sql
cloud.affinity_group_view.sql
cloud.async_job_view.sql
cloud.data_center_view.sql
cloud.disk_offering_view.sql
cloud.domain_router_view.sql
cloud.domain_view.sql
cloud.event_view.sql
cloud.free_ip_view.sql
cloud.host_view.sql
cloud.image_store_view.sql
cloud.instance_group_view.sql
cloud.last_annotation_view.sql
cloud.mshost_view.sql
cloud.network_offering_view.sql
cloud.project_account_view.sql
cloud.project_invitation_view.sql
cloud.project_view.sql
cloud.resource_tag_view.sql
cloud.security_group_view.sql
cloud.service_offering_view.sql
cloud.snapshot_view.sql
cloud.snapshot_view.sql
cloud.storage_pool_view.sql
cloud.storage_pool_view.sql
cloud.template_view.sql
cloud.template_view.sql
cloud.user_view.sql
cloud.user_view.sql
cloud.user_vm_view.sql
cloud.user_vm_view.sql
cloud.volume_view.sql
cloud.vpc_offering_view.sql

ACTUAL RESULTS

$ ls -1 engine/schema/src/main/resources/META-INF/db/views/
cloud.async_job_view.sql
cloud.data_center_view.sql
cloud.disk_offering_view.sql
cloud.domain_router_view.sql
cloud.host_view.sql
cloud.network_offering_view.sql
cloud.service_offering_view.sql
cloud.snapshot_view.sql
cloud.storage_pool_view.sql
cloud.template_view.sql
cloud.user_view.sql
cloud.user_vm_view.sql

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

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?

DaanHoogland · 2024-02-13T14:32:20Z

@GutoVeronezi , can you advice if this is all that is needed to put views under the new maintenance regime? I really only want volume_view for now, so is there any reason not to do them all right now?
thanks

engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql

codecov · 2024-02-13T14:42:04Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (bda49ab) 30.90% compared to head (928c5d8) 30.93%.

Additional details and impacted files

@@             Coverage Diff              @@
##               4.19    #8647      +/-   ##
============================================
+ Coverage     30.90%   30.93%   +0.03%     
- Complexity    34189    34224      +35     
============================================
  Files          5347     5347              
  Lines        375566   375566              
  Branches      54625    54625              
============================================
+ Hits         116068   116190     +122     
+ Misses       244236   244067     -169     
- Partials      15262    15309      +47

Flag	Coverage Δ
simulator-marvin-tests	`24.82% <ø> (+0.05%)`	⬆️
uitests	`4.39% <ø> (ø)`
unit-tests	`16.55% <ø> (+<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.

GutoVeronezi · 2024-02-13T14:44:53Z

@GutoVeronezi , can you advice if this is all that is needed to put views under the new maintenance regime? I really only want volume_view for now, so is there any reason not to do them all right now? thanks

@DaanHoogland
By the design we created, creating the view's file under the views directory should be enough.

PR #7417's main purpose was to introduce the concept. The idea was to create the new files as we would change the views; however, there is no problem in creating them right away.

DaanHoogland · 2024-02-13T14:52:33Z

PR #7417's main purpose was to introduce the concept.

I know, or better I remember now ;)

The idea was to create the new files as we would change the views; however, there is no problem in creating them right away.

In hindsight it might be a bit confusing and lead to review errors . I'll get myself in an agry streak soon and add the rest.

GutoVeronezi · 2024-02-13T14:54:33Z

In hindsight it might be a bit confusing and lead to review errors .

Indeed

weizhouapache · 2024-02-13T15:17:26Z

PR #7417's main purpose was to introduce the concept.

I know, or better I remember now ;)

The idea was to create the new files as we would change the views; however, there is no problem in creating them right away.

In hindsight it might be a bit confusing and lead to review errors . I'll get myself in an agry streak soon and add the rest.

@DaanHoogland @GutoVeronezi

IMHO, ideally the database upgrade process should be

drop procedures if exist, and create procedures
perform database upgrade scripts (in .sql files) and data migration (in .java files), which should not depend on views
create views

#7417 has introduced the concept to create views as last step, which is good.
however, we should also consider

recreating all views might take quite long time if the database if huge
the upgrade script or data migration might rely on some views (need double-check)

DaanHoogland · 2024-02-13T15:28:58Z

recreating all views might take quite long time if the database if huge

so we should add intelligence as to which views have changed

the upgrade script or data migration might rely on some views (need double-check)

well, this should not be a problem as all views are still there while other upgrades occur. I think it is the programmers responsibility to have their dependencies straight.

the first one is a valid consideration but out of scope here I think.

GutoVeronezi · 2024-02-13T16:30:18Z

recreating all views might take quite long time if the database if huge

View structures creating should not be affected by the database size (only running them). They are structures that represent queries; thus, not relying on the amount of data for its creation.

the upgrade script or data migration might rely on some views (need double-check)

Regarding this one, you can check my comment on PR #7417 (comment), which answers the same question you made.

weizhouapache · 2024-02-13T17:56:53Z

recreating all views might take quite long time if the database if huge

View structures should not be affected by the database size. They are structures that represent queries; thus, not relying on the amount of data.

good to know it.

the upgrade script or data migration might rely on some views (need double-check)

Regarding this one, you can check my comment on PR #7417 (comment), which answers the same question you made.

Yes, I remember it.
I am ok with moving all views to separated files. We need to test the upgrade of different cloudstack versions.

weizhouapache

code lgtm

DaanHoogland · 2024-02-14T16:41:18Z

@blueorangutan package

blueorangutan · 2024-02-14T16:42:03Z

@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 · 2024-02-14T17:45:02Z

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

DaanHoogland · 2024-02-15T08:53:07Z

@blueorangutan test alma9 kvm-alma9

blueorangutan · 2024-02-15T08:54:05Z

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

blueorangutan · 2024-02-15T23:30:12Z

[SF] Trillian test result (tid-9228)
Environment: kvm-alma9 (x2), Advanced Networking with Mgmt server a9
Total time taken: 50771 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8647-t9228-kvm-alma9.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_trigger_shutdown	`Failure`	341.27	test_safe_shutdown.py

yadvr

Refactoring makes sense

DaanHoogland · 2024-02-16T10:43:34Z

@blueorangutan test matrix

blueorangutan · 2024-02-16T10:44:03Z

@DaanHoogland a [SL] Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2024-02-16T23:37:17Z

[SF] Trillian test result (tid-9253)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 44956 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8647-t9253-xenserver-71.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_trigger_shutdown	`Failure`	336.79	test_safe_shutdown.py

blueorangutan · 2024-02-17T00:59:57Z

[SF] Trillian test result (tid-9254)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 49856 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8647-t9254-vmware-67u3.zip
Smoke tests completed. 128 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_02_balanced_drs_algorithm	`Error`	422.74	test_cluster_drs.py

blueorangutan · 2024-02-17T02:18:51Z

[SF] Trillian test result (tid-9255)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 54530 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8647-t9255-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

Fixes cloud.domain_view.sql added in apache#8647 Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

* volume view in separate file for easier modification * eof * remaining views * lintworm

volume view in separate file for easier modification

9ed98d1

DaanHoogland commented Feb 13, 2024

View reviewed changes

engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql Outdated Show resolved Hide resolved

eof

03d7030

remaining views

99578e5

DaanHoogland marked this pull request as ready for review February 13, 2024 15:29

lintworm

928c5d8

weizhouapache changed the title ~~volume view in separate file for easier modification~~ Move views into separate files for easier modification Feb 13, 2024

weizhouapache added this to the 4.19.1.0 milestone Feb 13, 2024

weizhouapache approved these changes Feb 13, 2024

View reviewed changes

DaanHoogland added status:needs-testing component:database labels Feb 14, 2024

DaanHoogland assigned vladimirpetrov Feb 14, 2024

yadvr approved these changes Feb 16, 2024

View reviewed changes

weizhouapache merged commit e9416c4 into apache:4.19 Feb 17, 2024

DaanHoogland deleted the 4.19-remainingViews branch February 19, 2024 08:37

weizhouapache mentioned this pull request Feb 19, 2024

list by isEncrypted #8643

Merged

13 tasks

shwstppr added a commit to shapeblue/cloudstack that referenced this pull request Feb 26, 2024

db: fix wrong cloud.domain_view

883b892

Fixes cloud.domain_view.sql added in apache#8647 Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

shwstppr mentioned this pull request Feb 26, 2024

engine-schema: fix wrong cloud.domain_view #8705

Merged

13 tasks

dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Mar 5, 2024

Move views into separate files for easier modification (apache#8647)

afe2352

* volume view in separate file for easier modification * eof * remaining views * lintworm

Move views into separate files for easier modification #8647

Move views into separate files for easier modification #8647

Uh oh!

Conversation

DaanHoogland commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

EXPECTED RESULTS

ACTUAL RESULTS

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!

DaanHoogland commented Feb 13, 2024

Uh oh!

Uh oh!

codecov bot commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

GutoVeronezi commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaanHoogland commented Feb 13, 2024

Uh oh!

GutoVeronezi commented Feb 13, 2024

Uh oh!

weizhouapache commented Feb 13, 2024

Uh oh!

DaanHoogland commented Feb 13, 2024

Uh oh!

GutoVeronezi commented Feb 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weizhouapache commented Feb 13, 2024

Uh oh!

weizhouapache left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Feb 14, 2024

Uh oh!

blueorangutan commented Feb 14, 2024

Uh oh!

blueorangutan commented Feb 14, 2024

Uh oh!

DaanHoogland commented Feb 15, 2024

Uh oh!

blueorangutan commented Feb 15, 2024

Uh oh!

blueorangutan commented Feb 15, 2024

Uh oh!

yadvr left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Feb 16, 2024

Uh oh!

blueorangutan commented Feb 16, 2024

Uh oh!

blueorangutan commented Feb 16, 2024

Uh oh!

blueorangutan commented Feb 17, 2024

Uh oh!

blueorangutan commented Feb 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DaanHoogland commented Feb 13, 2024 •

edited

Loading

codecov bot commented Feb 13, 2024 •

edited

Loading

GutoVeronezi commented Feb 13, 2024 •

edited

Loading

GutoVeronezi commented Feb 13, 2024 •

edited

Loading