You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by "DaanHoogland (via GitHub)" <gi...@apache.org> on 2024/02/13 09:08:28 UTC
[PR] list by isEncrypted [cloudstack]
DaanHoogland opened a new pull request, #8643:
URL: https://github.com/apache/cloudstack/pull/8643
### Description
This PR...
<!--- Describe your changes in DETAIL - And how has behaviour functionally changed. -->
<!-- For new features, provide link to FS, dev ML discussion etc. -->
<!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. -->
<!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged -->
<!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" -->
Fixes: #8635
very much a PoC on this.
<!--- ******************************************************************************* -->
<!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. -->
<!--- PLEASE PUT AN 'X' in only **ONE** box -->
<!--- ******************************************************************************* -->
### 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)
- [x] 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
- [x] Minor
#### Bug Severity
- [ ] BLOCKER
- [ ] Critical
- [x] Major
- [ ] Minor
- [ ] Trivial
### Screenshots (if appropriate):
### How Has This Been Tested?
<!-- Please describe in detail how you tested your changes. -->
<!-- Include details of your testing environment, and the tests you ran to -->
#### How did you try to break this feature and the system with this change?
<!-- see how your change affects other areas of the code, etc. -->
<!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2160018172
@kiranchavala a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2158388231
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2079153340
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2158517049
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9846
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#discussion_r1635828144
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ListVolumesCmd.java:
##########
@@ -96,6 +96,9 @@ public class ListVolumesCmd extends BaseListRetrieveOnlyResourceCountCmd impleme
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "state of the volume. Possible values are: Ready, Allocated, Destroy, Expunging, Expunged.")
private String state;
+ @Parameter(name = ApiConstants.IS_ENCRYPTED, type = CommandType.BOOLEAN, description = "list only volumes that are encrypted", since = "4.20",
Review Comment:
```suggestion
@Parameter(name = ApiConstants.IS_ENCRYPTED, type = CommandType.BOOLEAN, description = "list only volumes that are encrypted", since = "4.19.1",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2079226868
Since the query uses VolumeVO now, the fix could be much simple like
```
if (cmd.isEncrypted() != null) {
if (cmd.isEncrypted()) {
volumeSearchBuilder.and("encryptFormat", volumeSearchBuilder.entity().getEncryptFormat(), SearchCriteria.Op.NNULL);
} else {
volumeSearchBuilder.and("encryptFormat", volumeSearchBuilder.entity().getEncryptFormat(), SearchCriteria.Op.NULL);
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2154881111
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9821
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2085042112
> code looks good
>
> @DaanHoogland a suggestion, can you add encryptformat to the VolumeResponse ? (admin only)
We could do @weizhouapache , but as this is not touching the actual response object but only the selection criteria, it seems to be a separate issue to me. The Cmd type is not know at the time of Response creation. It goes two levels deep onto the stack in Helper classes. `ViewResponseHelper::createVolumeResponse(..)` > `ApiDBUtils::newVolumeResponse(..)/fillVolumeData(..)`. This unless we want to always add the encryption type of course. In that case it is no effort.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2124838307
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1952035610
@DaanHoogland
maybe better to add the column to `volume_view`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2085073967
> > code looks good
> > @DaanHoogland a suggestion, can you add encryptformat to the VolumeResponse ? (admin only)
>
> We could do @weizhouapache , but as this is not touching the actual response object but only the selection criteria, it seems to be a separate issue to me. The Cmd type is not know at the time of Response creation. It goes two levels deep onto the stack in Helper classes. `ViewResponseHelper::createVolumeResponse(..)` > `ApiDBUtils::newVolumeResponse(..)/fillVolumeData(..)`. This unless we want to always add the encryption type of course. In that case it is no effort.
ok @DaanHoogland
it is a little bit difficult to verify if a volume is encrypted in the listVolumes API reponse.
I just checked the volume_view, it does not have `encrypt_format` so may need to recreate the volume_view
let's fix it in another pr.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2162161280
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2124820519
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9673
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1953777532
> > > @DaanHoogland maybe better to add the column to `volume_view`
> >
> >
> > makes sense but the field doesn't exist. I'll refresh my sql-fu to get it in.
>
> with your PR #8647 , you need to add a line to engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql
>
> ```
> `volumes`.`encrypt_format` AS `encrypt_format`,
> ```
>
> then add the field in `VolumeJoinVO`
I thought you meant a field `isEncrypted`. this is simpler, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1952587033
> > @DaanHoogland maybe better to add the column to `volume_view`
>
> makes sense but the field doesn't exist. I'll refresh my sql-fu to get it in.
with your PR #8647 , you need to add a line to engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql
```
`volumes`.`encrypt_format` AS `encrypt_format`,
```
then add the field in `VolumeJoinVO`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2129386421
@DaanHoogland it would be good to have this filter in the UI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1940862683
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8643?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`9b18243`)](https://app.codecov.io/gh/apache/cloudstack/commit/9b18243b876b4c5df8a53a9935d14cd2d8fdf6df?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 30.74% compared to head [(`e1eeca8`)](https://app.codecov.io/gh/apache/cloudstack/pull/8643?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) 4.38%.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8643 +/- ##
============================================
- Coverage 30.74% 4.38% -26.37%
============================================
Files 5352 361 -4991
Lines 374460 28706 -345754
Branches 54610 5004 -49606
============================================
- Hits 115123 1258 -113865
+ Misses 244068 27309 -216759
+ Partials 15269 139 -15130
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.38% <ø> (ø)` | |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8643?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2162566570
Merging this based on the reviews & test results.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2162160072
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2084995115
code looks good
@DaanHoogland
a suggestion, can you add encryptformat to the VolumeResponse ? (admin only)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2161571088
<b>[SF] Trillian test result (tid-10410)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 45277 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8643-t10410-kvm-centos7.zip
Smoke tests completed. 131 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2158385869
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti merged PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2162250420
Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 9877
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2127069651
<b>[SF] Trillian test result (tid-10258)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 53578 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8643-t10258-kvm-centos7.zip
Smoke tests completed. 129 look OK, 2 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` | 421.82 | test_events_resource.py
test_05_rvpc_multi_tiers | `Failure` | 543.50 | test_vpc_redundant.py
test_05_rvpc_multi_tiers | `Error` | 543.52 | test_vpc_redundant.py
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2124674066
@blueorangutan package
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "weizhouapache (via GitHub)" <gi...@apache.org>.
weizhouapache commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1953951581
> > > > @DaanHoogland maybe better to add the column to `volume_view`
> > >
> > >
> > > makes sense but the field doesn't exist. I'll refresh my sql-fu to get it in.
> >
> >
> > with your PR #8647 , you need to add a line to engine/schema/src/main/resources/META-INF/db/views/cloud.volume_view.sql
> > ```
> > `volumes`.`encrypt_format` AS `encrypt_format`,
> > ```
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> >
> > then add the field in `VolumeJoinVO`
>
> I thought you meant a field `isEncrypted`. this is simpler, thanks.
if need, you can add a method `isEncrypted` in `VolumeJoinVO`
:smiley:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on code in PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#discussion_r1580864323
##########
server/src/main/java/com/cloud/api/query/QueryManagerImpl.java:
##########
@@ -2587,6 +2588,20 @@ private Pair<List<Long>, Integer> searchForVolumeIdsAndCount(ListVolumesCmd cmd)
return new Pair<>(vmIds, count);
}
+ private void checkEncrypted(ListVolumesCmd cmd, List<Long> ids) {
Review Comment:
It's better to add a conditional check for this. This will result in getting all the encrypted volumes from the database which can be an intensive operation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2079153488
## [Codecov](https://app.codecov.io/gh/apache/cloudstack/pull/8643?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
Attention: Patch coverage is `29.62963%` with `19 lines` in your changes are missing coverage. Please review.
> Project coverage is 30.74%. Comparing base [(`b93a86c`)](https://app.codecov.io/gh/apache/cloudstack/commit/b93a86c9da8ce759d2821f2fac3ef06021b7ebfd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`e1eeca8`)](https://app.codecov.io/gh/apache/cloudstack/pull/8643?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> :exclamation: Current head e1eeca8 differs from pull request most recent head 4b14ca4. Consider uploading reports for the commit 4b14ca4 to get more accurate results
| [Files](https://app.codecov.io/gh/apache/cloudstack/pull/8643?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...main/java/com/cloud/storage/dao/VolumeDaoImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8643?src=pr&el=tree&filepath=engine%2Fschema%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fstorage%2Fdao%2FVolumeDaoImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZW5naW5lL3NjaGVtYS9zcmMvbWFpbi9qYXZhL2NvbS9jbG91ZC9zdG9yYWdlL2Rhby9Wb2x1bWVEYW9JbXBsLmphdmE=) | 21.42% | [11 Missing :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8643?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ain/java/com/cloud/api/query/QueryManagerImpl.java](https://app.codecov.io/gh/apache/cloudstack/pull/8643?src=pr&el=tree&filepath=server%2Fsrc%2Fmain%2Fjava%2Fcom%2Fcloud%2Fapi%2Fquery%2FQueryManagerImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c2VydmVyL3NyYy9tYWluL2phdmEvY29tL2Nsb3VkL2FwaS9xdWVyeS9RdWVyeU1hbmFnZXJJbXBsLmphdmE=) | 33.33% | [7 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/cloudstack/pull/8643?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #8643 +/- ##
=============================================
+ Coverage 15.28% 30.74% +15.45%
- Complexity 11524 33042 +21518
=============================================
Files 5425 5352 -73
Lines 474016 374485 -99531
Branches 61139 54614 -6525
=============================================
+ Hits 72444 115122 +42678
+ Misses 393515 244089 -149426
- Partials 8057 15274 +7217
```
| [Flag](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [simulator-marvin-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `24.58% <29.62%> (?)` | |
| [uitests](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `4.38% <ø> (+0.11%)` | :arrow_up: |
| [unit-tests](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `16.41% <11.11%> (?)` | |
| [unittests](https://app.codecov.io/gh/apache/cloudstack/pull/8643/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/cloudstack/pull/8643?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1952405885
> @DaanHoogland maybe better to add the column to `volume_view`
makes sense but the field doesn't exist. I'll refresh my sql-fu to get it in.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "vishesh92 (via GitHub)" <gi...@apache.org>.
vishesh92 commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1956790910
@DaanHoogland Can we revisit this after we merge https://github.com/apache/cloudstack/pull/8321? This will probably result in conflicts with #8321.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-1952018482
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2124678341
@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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "blueorangutan (via GitHub)" <gi...@apache.org>.
blueorangutan commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2124842206
@DaanHoogland a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "DaanHoogland (via GitHub)" <gi...@apache.org>.
DaanHoogland commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2129362524
@vishesh92 can you re-re-view?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "kiranchavala (via GitHub)" <gi...@apache.org>.
kiranchavala commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2160015766
@blueorangutan test
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "sureshanaparti (via GitHub)" <gi...@apache.org>.
sureshanaparti commented on code in PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#discussion_r1635829733
##########
api/src/main/java/org/apache/cloudstack/api/command/user/volume/ListVolumesCmd.java:
##########
@@ -96,6 +96,9 @@ public class ListVolumesCmd extends BaseListRetrieveOnlyResourceCountCmd impleme
@Parameter(name = ApiConstants.STATE, type = CommandType.STRING, description = "state of the volume. Possible values are: Ready, Allocated, Destroy, Expunging, Expunged.")
private String state;
+ @Parameter(name = ApiConstants.IS_ENCRYPTED, type = CommandType.BOOLEAN, description = "list only volumes that are encrypted", since = "4.20",
Review Comment:
@DaanHoogland update since version if it's merged to 4.19
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] list by isEncrypted [cloudstack]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8643:
URL: https://github.com/apache/cloudstack/pull/8643#issuecomment-2159895641
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org