-
Notifications
You must be signed in to change notification settings - Fork 898
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
Targeted scope serialization #17408
Targeted scope serialization #17408
Conversation
Enhance ReferenceStorage so it's usable for TargetedScope
Turn targeted_scope into ReferenceStorage
Update usage of targeted_scope
Make targeted_scope serialization work
Fix typo in comment
Simplify to_hash serialization condition
Add guard for full reference building.
Make targeted_scope serialization non recursive for now
Test targeted_scope serialization works
@@ -74,7 +74,7 @@ def build_full_reference(data, keys) | |||
if data.kind_of?(Hash) | |||
data | |||
else | |||
# assert_index makes sure that only keys of size 1 can go here | |||
raise "Please provide Hash as a reference, :manager_ref count includes more then 1 attribute. keys: #{keys}, data: #{data}" if keys.size > 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo more then
-> more than
Checked commits Ladas/manageiq@fc2b8e8~...a0e65d1 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, pending approval of ManageIQ/manageiq-providers-kubevirt#68
Modeling targeted_scope as ReferenceStorage and making it serializable.
After merging this, we need to merge these to fix failing CI:
ManageIQ/manageiq-providers-kubevirt#68
ManageIQ/manageiq-providers-vmware#258