Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
skills: add basic grant-revoke methods #374
Conversation
zyga
referenced this pull request
Jan 25, 2016
Merged
skills: add basic methods for skill handing #369
zyga
added some commits
Jan 26, 2016
niemeyer
reviewed
Jan 29, 2016
| + skills map[string]map[string]*Skill | ||
| + slots map[string]map[string]*Slot | ||
| + skillUsedBySlot map[*Slot]map[*Skill]bool | ||
| + slotsUsingSkill map[*Skill]map[*Slot]bool |
niemeyer
reviewed
Jan 29, 2016
| + } | ||
| + // Ensure that skill and slot are compatible | ||
| + if slot.Type != skill.Type { | ||
| + return fmt.Errorf("cannot grant skill %q from snap %q to slot %q from snap %q, skill type %q doesn't match slot type %q", |
niemeyer
Jan 29, 2016
Contributor
In these cases I think we should go short-hand, with ":":
cannot grant skill "%s:%s" (skill type %q) to "%s:%s" (skill type %q)
The names were validated, so we can trust them to not contain funky chars.
niemeyer
reviewed
Jan 29, 2016
| + } | ||
| + // Ensure that slot and skill are not connected yet | ||
| + if r.skillUsedBySlot[slot][skill] { | ||
| + return fmt.Errorf("cannot grant skill %q from snap %q to slot %q from snap %q twice", |
niemeyer
Jan 29, 2016
Contributor
This should probably not be an error. The exact semantics the function call requested are already in place, so there's no reason to blow it up I think. We should just return nil, and the call sites will be happy that the state they want is active.
niemeyer
reviewed
Jan 29, 2016
| + skillName, skillSnapName, slotName, slotSnapName) | ||
| + } | ||
| + delete(r.skillUsedBySlot[slot], skill) | ||
| + delete(r.slotsUsingSkill[skill], slot) |
niemeyer
Jan 29, 2016
Contributor
As in skill and slot removals, the base maps themselves should have the keys deleted once the respective value is empty, so they don't grow out of bounds.
|
LGTM once you're happy about these points. |
zyga
added some commits
Feb 1, 2016
|
Refreshed, please have a 2nd look. |
|
|
zyga commentedJan 25, 2016
This branch adds the four basic grant / revoke methods
Repository.Grant() grants a skill to a slot.
Repository.Revoke() revokes an existing grant.
Repository.GrantedTo() returns information about all skills granted to particular slots in a given snap.
Repository.GrantedBy() returns information about all skills, from a particular snap, granted to various slots in various snaps.
There are several related internal changes. Removing used skills or
occupied slots is no longer allowed and now returns an error.
Internally tests were tweaked to ensure that test skill and test slot
don't belong to the same snap. This helps in making asymmetric tests for
granted-to vs granted-by easier to follow.