-
Notifications
You must be signed in to change notification settings - Fork 261
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core: avoid null value so the ${DiskName} will be injected #370
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aesteve-rh
approved these changes
May 12, 2022
bennyz
reviewed
May 12, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
09a7804
to
1e21ce7
Compare
sleviim
suggested changes
May 15, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
once you ack all comments, please press 'mark as resolved so the patch will get merged. |
sleviim
approved these changes
May 17, 2022
ahadas
approved these changes
May 18, 2022
ahadas
reviewed
May 18, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
ahadas
reviewed
May 23, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
ahadas
reviewed
May 23, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
barpavel
reviewed
May 23, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
/ost |
/OST |
/ost |
barpavel
reviewed
May 23, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
ahadas
reviewed
May 24, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Show resolved
Hide resolved
3b61feb
to
4e6345c
Compare
ahadas
reviewed
May 25, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Outdated
Show resolved
Hide resolved
aesteve-rh
reviewed
May 25, 2022
...anager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/MoveDiskCommand.java
Show resolved
Hide resolved
if somehow withoption function get null as variablevalue it will not try to inject variablevalue instead of variableName so this cause the message to look like this: Cannot remove Virtual Disk. Disk ${DiskName} is being moved or copied. so I added validation of getParameters().getNewAlias() and getParameters().getDiskAlias() and if both are null the LockMessage will be somthing more genaric and will look like this: Cannot remove Virtual Disk. Related operation is currently in progress. Please try again later. and if at list one of them is not null I can inject into the message so the message will look like this: Cannot remove Virtual Disk. Disk vm_disk is being moved or copied. Bug-Url: https://bugzilla.redhat.com/2039746 Signed-off-by: Artiom Divak <adivak@redhat.com>
ahadas
approved these changes
May 26, 2022
/ost |
barpavel
approved these changes
May 26, 2022
/ost |
2 similar comments
/ost |
/ost |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
if somehow withoption function get null as variablevalue it will not
try to inject variablevalue instead of variableName so this cause the message to
look like this:
Cannot remove Virtual Disk. Disk ${DiskName} is being moved or copied.
so I added validation of getParameters().getNewAlias() and getParameters().getDiskAlias()
and if both are null the LockMessage will be somthing more genaric and will look like this:
Cannot remove Virtual Disk. Related operation is currently in progress. Please try again later.
and if at list one of them is not null I can inject into the message so the message will look like this:
Cannot remove Virtual Disk. Disk vm_disk is being moved or copied.
Bug-Url: https://bugzilla.redhat.com/2039746
Signed-off-by: Artiom Divak adivak@redhat.com