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

restapi: prefer parentType over subclasses in getParent #320

Merged
merged 1 commit into from May 2, 2022

Conversation

bennyz
Copy link
Member

@bennyz bennyz commented May 1, 2022

Currently LinkHelper#getParent may return subclasses even if the parentType exists in the methods list. This makes the returned resource unpredictable as the method list does not have any guaranteed order.
To fix this, this patch tries to match parentType first, and only if it is not in the list it will fallback to a subclass.

In the bug the parent is Vm, but since Backup also has a reference to the Snapshot model it may be chosen over the suggested parentType because it's a subclass of Vm.

Bug-Url: https://bugzilla.redhat.com/2080728

Copy link
Member

@ahadas ahadas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall looks good, added minor comments

@ahadas ahadas added the storage label May 1, 2022
@oliel
Copy link
Collaborator

oliel commented May 2, 2022

Consider Arik's suggestions for name changes, I don't feel strongly about any of them, but in general this solution LGTM, approved

Currently LinkHelper#getParent may return subclasses even if the
parentType exists in the methods list.

This makes the returned resource unpredicatble as the method list does
not have any guaranteed order.

To fix this, this patch tries to match parentType first, and only if it
is not in the list it will fallback to a subclass.

Bug-Url: https://bugzilla.redhat.com/2080728
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
@bennyz
Copy link
Member Author

bennyz commented May 2, 2022

/ost

@bennyz
Copy link
Member Author

bennyz commented May 2, 2022

OST failed on UI

@bennyz bennyz merged commit 90f3662 into oVirt:master May 2, 2022
@bennyz bennyz deleted the use-suggested-parent branch May 2, 2022 15:35
@ahadas ahadas added this to the ovirt-4.5.1 milestone May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants