-
Notifications
You must be signed in to change notification settings - Fork 44
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 storage config #1854
Fix storage config #1854
Conversation
Pull Request Test Coverage Report for Build 12471458359Details
💛 - Coveralls |
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.
Although DBus stuff are not my thing, rationale and code looks good to me. If you tested it and it works, I guess you can go ahead.
Nevertheless, I have left a comment to be discussed in a near future, if needed.
@@ -67,7 +67,8 @@ def finish | |||
# | |||
# @return [Hash] | |||
def config | |||
serialized_config = dbus_object.GetConfig | |||
# Use storage iface to avoid collision with bootloader iface |
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.
Interesting. I wonder if it worth to always use the corresponding iface to avoid more errors like the one this commit fixes and get rid of specific comment.
Something to be discussed, not a blocker for this PR.
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.
yeah, it is zbus way to always use combination of service, interface and object. On other hand ruby-dbus use different way and allow access throught interface, but for simplicity if method in unique, it is added directly to object. My original fix change dbus_object to always return interface, but then mixins start failing as it uses some methods that is not in interface. (during test e.g. this mixins failing https://github.com/agama-project/agama/blob/master/service/lib/agama/dbus/clients/with_service_status.rb#L43 ).
Maybe it will be better to have beside dbus_object also dbus_interface method on which we should call all needed storage methods ( that is idea that I get after thinking a bit more about it after I have working solution and not enough time to test different one ).
Merging it as requested in the internal |
Problem
There is collision between #1848 and #1840 where bootloader adds another interface with Get/SetConfig on Storage service. This result that generic access no longer works and needs access via specific interface.
reported on slack https://suse.slack.com/archives/C02TLF25571/p1734924853266999
Solution
Change code to avoid ambitious code.
Testing