Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
interfaces: API additions for interface hooks #3119
Conversation
stolowski
added some commits
Mar 28, 2017
Some checks were not successful
stolowski
referenced this pull request
Mar 31, 2017
Closed
[WIP] interfaces/hooks: expose attrs to the interface API, snapctl enhancements (step #4) #3120
zyga
approved these changes
Mar 31, 2017
LGTM
If you agree about the suggestion to have Validate{Plug,Slot} optional like other interface methods I can follow up with a quick PR that does this.
| @@ -225,3 +225,13 @@ func (iface *BluezInterface) AutoConnect(*interfaces.Plug, *interfaces.Slot) boo | ||
| // allow what declarations allowed | ||
| return true | ||
| } | ||
| + | ||
| +func (iface *BluezInterface) ValidatePlug(plug *interfaces.Plug, attrs map[string]interface{}) error { |
zyga
Mar 31, 2017
Contributor
I kind of wish we did the same trick we have elsewhere with interfaces. We could have a function that validates a plug that checks if the interface supports validation. This would cut the amount of boilerplate code significantly as it could do simple stuff like checking if interfaces match etc etc (sanitize could be done the same way). What do you think?
stolowski
Mar 31, 2017
•
Edited 1 time
-
stolowski
Mar 31, 2017
Contributor
-
Mar 31, 2017
@zyga Ok, I made ValidatePlug/Slot optional in the PR #3120 (also improved a test there to make sure it's called when it should). ValidatePlug/Slot is removed from base Interface in core.go completely and I've removed it from all interfaces which have no attributes (so there is no potential use for Validate* to exists atm); I've left empty stubs for Validate* in interfaces which can potentially make use of attributes from hooks. This all makes sense in PR #3120 (I hope).
| @@ -131,21 +127,13 @@ func (iface *unity8PimCommonInterface) AppArmorConnectedPlug(spec *apparmor.Spec | ||
| return nil | ||
| } | ||
| -func (iface *unity8PimCommonInterface) ConnectedPlugSnippet(plug *interfaces.Plug, slot *interfaces.Slot, securitySystem interfaces.SecuritySystem) ([]byte, error) { |
stolowski
added some commits
Mar 31, 2017
Some checks were not successful
Some checks were not successful
mvo5
requested a review
from
niemeyer
Apr 3, 2017
stolowski
and others
added some commits
Apr 3, 2017
Some checks were not successful
All checks have passed
All checks have failed
All checks have failed
Some checks were not successful
All checks have passed
All checks have passed
All checks have failed
Some checks were not successful
Some checks were not successful
Some checks were not successful
|
Can we please...
... before landing this branch? |
stolowski
added some commits
May 10, 2017
All checks have passed
niemeyer
approved these changes
May 10, 2017
Nice, thanks! And thanks to @zyga as well for the tests and the care taken ensuring things remain working in the future.
stolowski commentedMar 31, 2017
•
Edited 1 time
-
stolowski
Mar 31, 2017
Changes to the interface of interfaces: extended AddConnectedPlug/Slot methods to take slot/plug attributes maps. Added stubs for ValidatePlug/Slot methods which will be called to validate dynamic attributes coming from interface hooks (this is implemented in PR #3120).
The API will be used in a followup PR.