Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
kernel plugin: introduce kernel-channel to select from which channel … #1720
Conversation
| - 'core', 'stable', self.os_snap, self.project.deb_arch) | ||
| + channel = self.options.kernel_channel | ||
| + logger.info('Downloading core from the {!r} channel'.format(channel)) | ||
| + if channel in ['stable', 'beta', 'candidate', 'edge']: |
kalikiana
Nov 9, 2017
Collaborator
These are all the channels, right? Perhaps this would make sense to handle in the download method for the benefit of all users of the API?
kyrofa
Nov 9, 2017
Member
Note that there are hotfix branches as well, that can be created at any time.
kalikiana
Nov 10, 2017
Collaborator
I can't tell if that's in favor of or against my suggestion. Is this list specific to the plugin or is any valid channel recognized by the store okay, and as such should be checked in the store API?
kyrofa
Nov 13, 2017
•
Member
Neither, it's in response to "these are all the channels, right?"
I sort of feel like we shouldn't be checking them here, but simply attempting to download the channel provided. That means this will work for hotfix branches as well, and also seems more pythonic to simply catch any errors tossed by snapcraft.download instead of checking the channels first. @piso77 do you get a reasonable error if you try to download from a non-existent branch?
piso77
Nov 14, 2017
Contributor
Indeed, passing an invalid channel already raises an error:
...
Downloading core from the 'foobar' channel
Snap 'core' for 'amd64' cannot be found in the 'foobar' channel.
I guess, i can remove the channel checks from the kernel plugin and the unit tests too, since it's already handled by the download API.
| @@ -1078,9 +1079,21 @@ def test_pull(self, download_mock): | ||
| plugin.pull() | ||
| download_mock.assert_called_once_with( | ||
| - 'core', 'stable', plugin.os_snap, | ||
| + 'core', self.options.kernel_channel, plugin.os_snap, |
kalikiana
Nov 10, 2017
Collaborator
By scenarios I actually meant a list of values used to run the same test. Like what's done in KernelPluginDefaulTargetsTestCase. Sorry if that wasn't very clear.
|
(there are conflicts here) |
piso77 commentedNov 8, 2017
…core should be downloaded
Since "aa8a842 kernel plugin: use latest stable core snap (#1624)" the kernel plugin forcefully download core from the stable channel (while previously that was the edge channel).
Introduce the ability to let the user choose which channel to use via the 'kernel_channel' keyword:
If not specified, the kernel plugin will default to use 'stable', so no semantic changes are introduced.
https://bugs.launchpad.net/snapcraft/+bug/1730948
Signed-off-by: Paolo Pisati paolo.pisati@canonical.com