Skip to content
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

fix: SerializationException: Type 'org.ovirt.engine.core.common.utils.SecretValue' was not included in the set of types... #900 #956

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

sermakov-orion
Copy link
Contributor

@sermakov-orion sermakov-orion commented Aug 2, 2024

Fixes issue

This PR fixes the #900 issue.

Changes introduced with this PR

Since the SecretValue structure is used in GWT, it should implement Serializable interface. Also couple null checks were added to avoid NPE in the scenario explained in the #900 issue.

Are you the owner of the code you are sending in, or do you have permission of the owner?

Yes

@sermakov-orion
Copy link
Contributor Author

Requesting Reviews.

@tomglx
Copy link

tomglx commented Dec 12, 2024

Any Idea when this will be committed to the master branch?

@JasperB-TeamBlue
Copy link
Contributor

JasperB-TeamBlue commented Jan 9, 2025

Looks good to me, it solves the issue related to it.

##Fixes issue
This PR fixes the oVirt#900 issue.

##Changes introduced with this PR
Since the SecretValue structure is used in GWT, it should implement Serializable interface.
Also couple null checks were added to avoid NPE in the scenario explained in the oVirt#900 issue.

##Are you the owner of the code you are sending in, or do you have permission of the owner?
Yes
@dupondje dupondje force-pushed the npe-snapshot-screen branch from 31ef9f7 to 22f6205 Compare January 9, 2025 12:26
@dupondje dupondje merged commit 36a3073 into oVirt:master Jan 9, 2025
2 checks passed
@dupondje
Copy link
Member

dupondje commented Jan 9, 2025

@sermakov-orion : Thanks for the patch!
Please have some proper commit message in the future :) Cause now it was a bit messed up (fixed in the merge).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants