Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/new-snap-select-processors into lp:launchpad
* Also, moved the processors selector up, between series and upload to store. -- https://code.launchpad.net/~maxiberta/launchpad/new-snap-select-processors/+merge/298242 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/new-snap-select-processors into lp:launchpad
* Check all available processors by default.
* Dropped `SnapSet.availableProcessors()` for populating the processors widget,
in favor of `getUtility(IProcessorSet).getAll()` to keep the same logic as
`SnapSet.new()`.
* Indent processor checkboxes to keep them aligned with the series selector.
* Fixed a typo in forms CSS.
Thanks!
Diff comments:
> === modified file 'lib/canonical/launchpad/icing/css/forms.css'
> --- lib/canonical/launchpad/icing/css/forms.css 2012-08-30 02:40:04
> +
> +++ lib/canonical/launchpad/icing/css/forms.css 2016-06-30 19:00:43
> +
> @@ -55,7 +56,7 @@
> display: block;
> margin-top: 4px;
> }
> -inout[type=input] + button,
> +input[type=input] + button,
This was most likely a typo; no other references to it.
> img[src$=spinner] + button {
> margin-left: 6px !important;
> }
--
https://code.launchpad.net/~maxiberta/launchpad/new-snap-select-processors/+merge/298242
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-channels-ui into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-channels-ui into
lp:launchpad with lp:~cjwatson/launchpad/snap-channels-job as a prerequisite.
Commit message:
Add UI for automatically releasing snap packages.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1597819 in Launchpad itself: "Add option to automatically release snap
packages to channels after upload"
https://bugs.launchpad.net/launchpad/+bug/1597819
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-channels-ui/+merge/298811
Add UI for automatically releasing snap packages.
This will require firewall changes first to let the appservers talk to
search.apps.ubuntu.com (although there's a feature flag in the
snap-channels-store-client branch so that can be disabled if something goes
wrong).
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/snap-channels-ui into lp:launchpad.
=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py 2016-06-30 17:23:30 +
+++ lib/lp/snappy/browser/snap.py 2016-06-30 17:23:30 +
@@ -332,6 +332,9 @@
# This is only required if store_upload is True. Later validation takes
# care of adjusting the required attribute.
store_name = copy_field(ISnap['store_name'], required=True)
+store_channels = copy_field(
+ISnap['store_channels'],
+value_type=Choice(vocabulary='SnapStoreChannel'), required=True)
def log_oops(error, request):
@@ -368,9 +371,11 @@
'auto_build_pocket',
'store_upload',
'store_name',
+'store_channels',
]
custom_widget('store_distro_series', LaunchpadRadioWidget)
custom_widget('auto_build_archive', SnapArchiveWidget)
+custom_widget('store_channels', LabeledMultiCheckBoxWidget)
def initialize(self):
"""See `LaunchpadView`."""
@@ -443,6 +448,7 @@
super(SnapAddView, self).validate_widgets(data, ['store_upload'])
store_upload = data.get('store_upload', False)
self.widgets['store_name'].context.required = store_upload
+self.widgets['store_channels'].context.required = store_upload
super(SnapAddView, self).validate_widgets(data, names=names)
@action('Create snap package', name='create')
@@ -464,7 +470,8 @@
auto_build_pocket=data['auto_build_pocket'],
private=private, store_upload=data['store_upload'],
store_series=data['store_distro_series'].snappy_series,
-store_name=data['store_name'], **kwargs)
+store_name=data['store_name'],
+store_channels=data.get('store_channels'), **kwargs)
if data['store_upload']:
self.requestAuthorization(snap)
else:
@@ -534,6 +541,7 @@
data, ['store_upload'])
store_upload = data.get('store_upload', False)
self.widgets['store_name'].context.required = store_upload
+self.widgets['store_channels'].context.required = store_upload
super(BaseSnapEditView, self).validate_widgets(data, names=names)
def _needStoreReauth(self, data):
@@ -572,6 +580,8 @@
if not store_upload:
if 'store_name' in data:
del data['store_name']
+if 'store_channels' in data:
+del data['store_channels']
need_store_reauth = self._needStoreReauth(data)
self.updateContextFromData(data)
if need_store_reauth:
@@ -631,8 +641,10 @@
'auto_build_pocket',
'store_upload',
'store_name',
+'store_channels',
]
custom_widget('store_distro_series', LaunchpadRadioWidget)
+custom_widget('store_channels', LabeledMultiCheckBoxWidget)
custom_widget('vcs', LaunchpadRadioWidget)
custom_widget('git_ref', GitRefWidget)
custom_widget('auto_build_archive', SnapArchiveWidget)
=== modified file 'lib/lp/snappy/browser/snapbuild.py'
--- lib/lp/snappy/browser/snapbuild.py 2016-06-21 14:51:06 +
+++ lib/lp/snappy/browser/snapbuild.py 2016-06-30 17:23:30 +
@@ -95,6 +95,11 @@
return structured(
'Manage this package in the store',
job.store_url)
+elif job.store_url:
+return structured(
+'Manage this package in the store'
+'Releasing package to channels failed: %s',
+job.store_url, job.error_message)
else:
return structured("Store upload failed: %s", job.error_message)
=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py 2016-06-30 17:23:30 +
+++ lib/lp/snappy/browser/tests/test_snap.py 2016-06-30 17:23:30 +
@@ -65,6 +65,7 @@
SnapFeatureDisabled,
SnapPrivateFeatureDisabled,
)
+from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
from
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-channels-job into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-channels-job into
lp:launchpad with lp:~cjwatson/launchpad/snap-channels-store-client as a
prerequisite.
Commit message:
Automatically release snap packages that have store_channels set.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1597819 in Launchpad itself: "Add option to automatically release snap
packages to channels after upload"
https://bugs.launchpad.net/launchpad/+bug/1597819
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-channels-job/+merge/298810
Automatically release snap packages that have store_channels set.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/snap-channels-job into lp:launchpad.
=== added file 'lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt'
--- lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt 1970-01-01 00:00:00 +
+++ lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt 2016-06-30 17:00:36 +
@@ -0,0 +1,5 @@
+This snap package could not be released automatically because it was held
+for manual review. Once it has been approved, you will need to release it
+manually from here:
+
+ %(store_url)s
=== added file 'lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt'
--- lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt 1970-01-01 00:00:00 +
+++ lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt 2016-06-30 17:00:36 +
@@ -0,0 +1,7 @@
+Launchpad asked the store to release this snap package, but it failed:
+
+ %(store_error_message)s
+
+You can try to release it manually here:
+
+ %(store_url)s
=== modified file 'lib/lp/snappy/mail/snapbuild.py'
--- lib/lp/snappy/mail/snapbuild.py 2016-06-21 14:51:06 +
+++ lib/lp/snappy/mail/snapbuild.py 2016-06-30 17:00:36 +
@@ -60,6 +60,34 @@
config.canonical.noreply_from_address,
"snap-build-upload-scan-failed", build)
+@classmethod
+def forManualReview(cls, build):
+"""Create a mailer for notifying about manual review.
+
+:param build: The relevant build.
+"""
+requester = build.requester
+recipients = {requester: RecipientReason.forBuildRequester(requester)}
+return cls(
+"%(snap_name)s held for manual review",
+"snapbuild-manualreview.txt", recipients,
+config.canonical.noreply_from_address,
+"snap-build-release-manual-review", build)
+
+@classmethod
+def forReleaseFailure(cls, build):
+"""Create a mailer for notifying about store release failures.
+
+:param build: The relevant build.
+"""
+requester = build.requester
+recipients = {requester: RecipientReason.forBuildRequester(requester)}
+return cls(
+"Store release failed for %(snap_name)s",
+"snapbuild-releasefailed.txt", recipients,
+config.canonical.noreply_from_address,
+"snap-build-release-failed", build)
+
def __init__(self, subject, template_name, recipients, from_address,
notification_type, build):
super(SnapBuildMailer, self).__init__(
@@ -77,10 +105,12 @@
"""See `BaseMailer`."""
build = self.build
upload_job = build.store_upload_jobs.first()
-if upload_job is None or upload_job.error_message is None:
+if upload_job is None:
error_message = ""
+store_url = ""
else:
-error_message = upload_job.error_message
+error_message = upload_job.error_message or ""
+store_url = upload_job.store_url or ""
params = super(SnapBuildMailer, self)._getTemplateParams(
email, recipient)
params.update({
@@ -100,6 +130,7 @@
"snap_authorize_url": canonical_url(
build.snap, view_name="+authorize"),
"store_error_message": error_message,
+"store_url": store_url,
})
if build.duration is not None:
duration_formatter = DurationFormatterAPI(build.duration)
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2016-06-21 14:51:06 +
+++ lib/lp/snappy/model/snapbuildjob.py 2016-06-30 17:00:36 +
@@ -50,8 +50,10 @@
ISnapStoreUploadJobSource,
)
from lp.snappy.interfaces.snapstoreclient import (
+BadReleaseResponse,
BadScanStatusResponse,
ISnapStoreClient,
+ReleaseFailedResponse,
ScanFailedResponse,
UnauthorizedUploadResponse,
UploadNotScannedYetResponse,
@@ -157,6 +159,10 @@
return oops_vars
+class ManualReview(Exception):
+pass
+
+
@implementer(ISnapStoreUploadJob)
@provider(ISnapStoreUploadJobSource)
class SnapStoreUploadJob(SnapBuildJobDerived):
@@ -164,7 +170,12 @@
class_job_type = SnapBuildJobType.STORE_UPLOAD
-user_error_types =
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-channels-store-client into lp:launchpad
Colin Watson has proposed merging
lp:~cjwatson/launchpad/snap-channels-store-client into lp:launchpad with
lp:~cjwatson/launchpad/snap-auto-build-ui as a prerequisite.
Commit message:
Add model and store client support for automatically releasing snap packages.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1597819 in Launchpad itself: "Add option to automatically release snap
packages to channels after upload"
https://bugs.launchpad.net/launchpad/+bug/1597819
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-channels-store-client/+merge/298806
Add model and store client support for automatically releasing snap packages.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/snap-channels-store-client into lp:launchpad.
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2016-05-18 00:33:18 +
+++ configs/development/launchpad-lazr.conf 2016-06-30 16:26:55 +
@@ -190,6 +190,7 @@
builder_proxy_auth_api_endpoint: http://snap-proxy.launchpad.dev:8080/tokens
builder_proxy_host: snap-proxy.launchpad.dev
builder_proxy_port: 3128
+store_search_url: https://search.apps.ubuntu.com/
tools_source: deb http://ppa.launchpad.net/snappy-dev/snapcraft-daily/ubuntu %(series)s main
[txlongpoll]
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2016-06-20 20:18:11 +
+++ lib/lp/services/config/schema-lazr.conf 2016-06-30 16:26:55 +
@@ -1804,6 +1804,9 @@
# The store's upload URL endpoint.
store_upload_url: none
+# The store's search URL endpoint.
+store_search_url: none
+
[process-job-source-groups]
# This section is used by cronscripts/process-job-source-groups.py.
dbuser: process-job-source-groups
=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py 2016-06-20 20:32:36 +
+++ lib/lp/snappy/interfaces/snap.py 2016-06-30 16:26:55 +
@@ -435,6 +435,13 @@
"Serialized secrets issued by the store and the login service to "
"authorize uploads of this snap package."))
+store_channels = List(
+value_type=TextLine(), title=_("Store channels"),
+required=False, readonly=False,
+description=_(
+"Channels to which to release this snap package after "
+"uploading it to the store."))
+
class ISnapAdminAttributes(Interface):
"""`ISnap` attributes that can be edited by admins.
=== modified file 'lib/lp/snappy/interfaces/snapstoreclient.py'
--- lib/lp/snappy/interfaces/snapstoreclient.py 2016-06-21 14:51:06 +
+++ lib/lp/snappy/interfaces/snapstoreclient.py 2016-06-30 16:26:55 +
@@ -8,11 +8,14 @@
__metaclass__ = type
__all__ = [
'BadRefreshResponse',
+'BadReleaseResponse',
'BadRequestPackageUploadResponse',
'BadScanStatusResponse',
+'BadSearchResponse',
'BadUploadResponse',
'ISnapStoreClient',
'NeedsRefreshResponse',
+'ReleaseFailedResponse',
'ScanFailedResponse',
'UnauthorizedUploadResponse',
'UploadNotScannedYetResponse',
@@ -53,6 +56,18 @@
pass
+class BadSearchResponse(Exception):
+pass
+
+
+class BadReleaseResponse(Exception):
+pass
+
+
+class ReleaseFailedResponse(Exception):
+pass
+
+
class ISnapStoreClient(Interface):
"""Interface for the API provided by the snap store."""
@@ -91,6 +106,26 @@
scanned the upload.
:raises BadScanStatusResponse: if the store failed to scan the
upload.
-:return: A URL on the store with further information about this
-upload.
+:return: A tuple of (`url`, `revision`), where `url` is a URL on the
+store with further information about this upload, and `revision`
+is the store revision number for the upload or None.
+"""
+
+def listChannels():
+"""Fetch the current list of channels from the store.
+
+:raises BadSearchResponse: if the attempt to fetch the list of
+channels from the store fails.
+:return: A list of dictionaries, one per channel, each of which
+contains at least "name" and "display_name" keys.
+"""
+
+def release(snapbuild, revision):
+"""Tell the store to release a snap build to specified channels.
+
+:param snapbuild: The `ISnapBuild` to release.
+:param revision: The revision returned by the store when uploading
+the build.
+:raises BadReleaseResponse: if the store failed to release the
+build.
"""
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2016-06-30 16:09:25 +
+++ lib/lp/snappy/model/snap.py 2016-06-30 16:26:55 +
@@ -187,12 +187,14 @@
store_secrets = JSON('store_secrets', allow_none=True)
+store_channels = JSON('store_channels',
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/db-snap-channels into lp:launchpad/db-devel
Colin Watson has proposed merging lp:~cjwatson/launchpad/db-snap-channels into lp:launchpad/db-devel. Commit message: Add Snap.store_channels column. Requested reviews: Launchpad code reviewers (launchpad-reviewers): db Related bugs: Bug #1597819 in Launchpad itself: "Add option to automatically release snap packages to channels after upload" https://bugs.launchpad.net/launchpad/+bug/1597819 For more details, see: https://code.launchpad.net/~cjwatson/launchpad/db-snap-channels/+merge/298803 Add Snap.store_channels column listing the channels (if any) to which a snap package should be released after a successful upload. This will just be a JSON-encoded array; we don't need anything more complicated. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/db-snap-channels into lp:launchpad/db-devel. === added file 'database/schema/patch-2209-69-5.sql' --- database/schema/patch-2209-69-5.sql 1970-01-01 00:00:00 + +++ database/schema/patch-2209-69-5.sql 2016-06-30 16:10:32 + @@ -0,0 +1,10 @@ +-- Copyright 2016 Canonical Ltd. This software is licensed under the +-- GNU Affero General Public License version 3 (see the file LICENSE). + +SET client_min_messages=ERROR; + +ALTER TABLE Snap ADD COLUMN store_channels text; + +COMMENT ON COLUMN Snap.store_channels IS 'Channels to which to release this snap package after uploading it to the store.'; + +INSERT INTO LaunchpadDatabaseRevision VALUES (2209, 69, 5); ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-auto-build-code into lp:launchpad
The proposal to merge lp:~cjwatson/launchpad/snap-auto-build-code into lp:launchpad has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-code/+merge/297957 -- Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-auto-build-model into lp:launchpad
The proposal to merge lp:~cjwatson/launchpad/snap-auto-build-model into lp:launchpad has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-model/+merge/297955 -- Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-auto-build-model into lp:launchpad
Diff comments:
>
> === modified file 'lib/lp/snappy/model/snap.py'
> --- lib/lp/snappy/model/snap.py 2016-05-28 00:21:40 +
> +++ lib/lp/snappy/model/snap.py 2016-06-29 15:47:32 +
> @@ -663,6 +689,55 @@
> list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
> person_ids, need_validity=True))
>
> +@staticmethod
> +def _findStaleSnaps():
> +"""See `ISnapSet`."""
> +threshold_date = (
> +datetime.now(pytz.UTC) -
> +timedelta(minutes=config.snappy.auto_build_frequency))
> +origin = [
> +Snap,
> +LeftJoin(
> +SnapBuild,
> +And(
> +SnapBuild.snap_id == Snap.id,
> +SnapBuild.archive_id == Snap.auto_build_archive_id,
> +SnapBuild.pocket == Snap.auto_build_pocket,
> +# We only want Snaps that haven't had an automatic
> +# SnapBuild dispatched for them recently.
> +SnapBuild.date_created >= threshold_date)),
> +]
> +return IStore(Snap).using(*origin).find(
> +Snap,
> +Snap.is_stale == True,
> +Snap.auto_build == True,
> +SnapBuild.date_created == None).config(distinct=True)
Yeah, nor do I (absent fussy DB changes to keep track of whether a build was
requested automatically or not). It should be reasonable in the common case,
at least, and the worst case here is that it takes an hour longer to dispatch
the automatic build.
> +
> +@classmethod
> +def makeAutoBuilds(cls, logger=None):
> +"""See `ISnapSet`."""
> +snaps = cls._findStaleSnaps()
> +builds = []
> +for snap in snaps:
> +snap.is_stale = False
> +if logger is not None:
> +logger.debug(
> +"Scheduling builds of snap package %s/%s",
> +snap.owner.name, snap.name)
> +for arch in snap.getAllowedArchitectures():
> +try:
> +build = snap.requestBuild(
> +snap.owner, snap.auto_build_archive, arch,
> +snap.auto_build_pocket)
> +if logger is not None:
> +logger.debug(
> +" - %s: Build requested.", arch.architecturetag)
> +builds.append(build)
> +except Exception as e:
> +if logger is not None:
> +logger.debug(" - %s: %s", arch.architecturetag, e)
> +return builds
> +
> def detachFromBranch(self, branch):
> """See `ISnapSet`."""
> self.findByBranch(branch).set(
--
https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-model/+merge/297955
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/new-snap-select-processors into lp:launchpad
Review: Needs Fixing Diff comments: > === modified file 'lib/lp/snappy/browser/snap.py' > --- lib/lp/snappy/browser/snap.py 2016-05-28 00:21:40 + > +++ lib/lp/snappy/browser/snap.py 2016-06-29 21:46:52 + > @@ -369,6 +370,16 @@ > self.context.information_type in PRIVATE_INFORMATION_TYPES): > raise SnapPrivateFeatureDisabled > > +def setUpFields(self): > +"""See `LaunchpadFormView`.""" > +super(SnapAddView, self).setUpFields() > +processors = getUtility(ISnapSet).availableProcessors() > +self.form_fields += self.createEnabledProcessors( > +processors, > +u"The architectures that this snap package builds for. Some " > +u"architectures are restricted and may only be enabled or " > +u"disabled by administrators.") > + I'm not sure that anything arranges for this form field to have sensible initial values; I haven't actually run it, but it looks to me as though all the processors will come up unchecked, and that seems to be what your tests require. All build_by_default processors should be checked by default, matching the behaviour of SnapSet.new when processors is None. > @property > def cancel_url(self): > return canonical_url(self.context) > @@ -636,7 +648,7 @@ > data['processors'].append(processor) > elif processor.name in widget.disabled_items: > # This processor is restricted and currently > -# enabled. Leave it untouched. > +# enabled. Leave it untouched. Grumble :-) > data['processors'].append(processor) > > > > === modified file 'lib/lp/snappy/interfaces/snap.py' > --- lib/lp/snappy/interfaces/snap.py 2016-05-28 00:21:40 + > +++ lib/lp/snappy/interfaces/snap.py 2016-06-29 21:46:52 + > @@ -543,3 +543,8 @@ > > This only exists to keep lazr.restful happy. > """ > + > +def availableProcessors(): Method names should be verbs in most cases, so I'd spell this "getAvailableProcessors". > +"""Return all architectures that are available to be enabled or > +disabled for new snap packages.") Superfluous '")' at the end there. > +""" > > === modified file 'lib/lp/snappy/model/snap.py' > --- lib/lp/snappy/model/snap.py 2016-05-28 00:21:40 + > +++ lib/lp/snappy/model/snap.py 2016-06-29 21:46:52 + > @@ -676,3 +676,12 @@ > def empty_list(self): > """See `ISnapSet`.""" > return [] > + > +@classmethod > +def availableProcessors(self): > +"""See `ISnapSet`.""" > +store = IMasterStore(Snap) This is a read-only query, so there's no need to force it onto the master; also a little odd to ask for the store for a different table, although that part won't matter in practice. Use IStore(Processor) instead. > +processors = store.find( > +Processor, > +Processor.id == DistroArchSeries.processor_id) > +return processors.config(distinct=True) > > === modified file 'lib/lp/snappy/templates/snap-new.pt' > --- lib/lp/snappy/templates/snap-new.pt 2016-05-24 05:15:50 + > +++ lib/lp/snappy/templates/snap-new.pt 2016-06-29 21:46:52 + > @@ -49,6 +49,13 @@ > > > > + > + As noted on IRC, this is a bit odd - I'm not sure you're allowed a br element as the immediate child of a table. If it's for visual adjustment, CSS on the following row would be better. Continuing to use the widget_row macro would be strongly preferable, so you'll have to work out how to push that down into the macro; I'd probably look around for some suitable existing class in lib/canonical/launchpad/icing/ if there is one and add one if there isn't, and then set widget_class on the widget in the corresponding browser code. > + > + > + > + > + > > > -- https://code.launchpad.net/~maxiberta/launchpad/new-snap-select-processors/+merge/298242 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
Review: Approve
Diff comments:
>
> === modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
> --- lib/lp/code/browser/tests/test_branchlisting.py 2015-12-11 04:40:07
> +
> +++ lib/lp/code/browser/tests/test_branchlisting.py 2016-06-28 21:11:44
> +
> @@ -389,7 +387,6 @@
> page = self.get_branch_list_page()
> self.assertThat(page, Not(snaps_matcher))
> with feature_flags():
I think you can drop this context manager block, and also adjust the comment at
the top of this method.
> -set_feature_flag(SNAP_FEATURE_FLAG, u'on')
> page = self.get_branch_list_page()
> if IPerson.providedBy(self.default_target):
> self.assertThat(page, snaps_matcher)
>
> === modified file 'lib/lp/snappy/browser/hassnaps.py'
> --- lib/lp/snappy/browser/hassnaps.py 2016-02-05 14:36:18 +
> +++ lib/lp/snappy/browser/hassnaps.py 2016-06-28 21:11:44 +
> @@ -42,12 +41,9 @@
> # Only enabled if the general snap feature flag is enabled
> # for public contexts and additionally if the snap_private
> # flag is enabled for private contexts.
This comment is no longer quite accurate.
> -if not bool(getFeatureFlag(SNAP_FEATURE_FLAG)):
> -enabled = False
> -else:
> -enabled = (
> -not self.context.private or
> -bool(getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG)))
> +enabled = (
> +not self.context.private or
> +bool(getFeatureFlag(SNAP_PRIVATE_FEATURE_FLAG)))
>
> text = 'Create snap package'
> return Link('+new-snap', text, enabled=enabled, icon='add')
> @@ -66,9 +62,7 @@
>
> @property
> def show_snap_information(self):
> -return (
> -bool(getFeatureFlag(SNAP_FEATURE_FLAG)) or
> -not self.snaps.is_empty())
> +return not self.snaps.is_empty()
This is only used in one place. I'd be inclined to inline it now (i.e.
tal:condition="not: view/snaps/is_empty").
>
> @property
> def snaps_link(self):
>
> === modified file 'lib/lp/snappy/browser/tests/test_snap.py'
> --- lib/lp/snappy/browser/tests/test_snap.py 2016-05-28 00:21:40 +
> +++ lib/lp/snappy/browser/tests/test_snap.py 2016-06-28 21:11:44 +
> @@ -137,7 +128,6 @@
> branch = self.factory.makeAnyBranch(
> owner=owner, information_type=InformationType.USERDATA)
> with feature_flags():
Is this context manager still needed?
> -set_feature_flag(SNAP_FEATURE_FLAG, u'on')
> with person_logged_in(owner):
> self.assertRaises(
> SnapPrivateFeatureDisabled, create_initialized_view,
>
> === modified file 'lib/lp/snappy/browser/tests/test_snaplisting.py'
> --- lib/lp/snappy/browser/tests/test_snaplisting.py 2016-05-18 12:15:05
> +
> +++ lib/lp/snappy/browser/tests/test_snaplisting.py 2016-06-28 21:11:44
> +
> @@ -39,8 +37,7 @@
> We do things this way rather than by calling self.useFixture because
> opening a URL in a test browser loses the thread-local feature flag.
> """
> -with FeatureFixture({SNAP_FEATURE_FLAG: u"on"}):
> -return self.factory.makeSnap(**kwargs)
> +return self.factory.makeSnap(**kwargs)
The method docstring is no longer accurate. At this point this method is no
longer pulling its weight anyway, so just change self.makeSnap to
self.factory.makeSnap throughout this class and delete the method.
>
> def assertSnapsLink(self, context, link_text, link_has_context=False,
> **kwargs):
>
> === modified file 'lib/lp/snappy/tests/test_snap.py'
> --- lib/lp/snappy/tests/test_snap.py 2016-05-17 12:47:34 +
> +++ lib/lp/snappy/tests/test_snap.py 2016-06-28 21:11:44 +
> @@ -86,18 +83,10 @@
>
> layer = LaunchpadZopelessLayer
>
> -def test_feature_flag_disabled(self):
> -# Without a feature flag, we will not create new Snaps.
> -person = self.factory.makePerson()
> -self.assertRaises(
> -SnapFeatureDisabled, getUtility(ISnapSet).new,
> -person, person, None, None, branch=self.factory.makeAnyBranch())
> -
> def test_private_feature_flag_disabled(self):
> # Without a private feature flag, we will not create new private
> Snaps.
> person = self.factory.makePerson()
> with feature_flags():
Is this context manager still needed?
> -set_feature_flag(SNAP_FEATURE_FLAG, u'on')
> self.assertRaises(
> SnapPrivateFeatureDisabled, getUtility(ISnapSet).new,
> person, person, None, None,
--
https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
___
Mailing list:
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-auto-build-code into lp:launchpad
Review: Approve code -- https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-code/+merge/297957 Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-auto-build-model into lp:launchpad
Review: Approve code
Diff comments:
>
> === modified file 'lib/lp/snappy/model/snap.py'
> --- lib/lp/snappy/model/snap.py 2016-05-28 00:21:40 +
> +++ lib/lp/snappy/model/snap.py 2016-06-29 15:47:32 +
> @@ -663,6 +689,55 @@
> list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
> person_ids, need_validity=True))
>
> +@staticmethod
> +def _findStaleSnaps():
> +"""See `ISnapSet`."""
> +threshold_date = (
> +datetime.now(pytz.UTC) -
> +timedelta(minutes=config.snappy.auto_build_frequency))
> +origin = [
> +Snap,
> +LeftJoin(
> +SnapBuild,
> +And(
> +SnapBuild.snap_id == Snap.id,
> +SnapBuild.archive_id == Snap.auto_build_archive_id,
> +SnapBuild.pocket == Snap.auto_build_pocket,
> +# We only want Snaps that haven't had an automatic
> +# SnapBuild dispatched for them recently.
> +SnapBuild.date_created >= threshold_date)),
> +]
> +return IStore(Snap).using(*origin).find(
> +Snap,
> +Snap.is_stale == True,
> +Snap.auto_build == True,
> +SnapBuild.date_created == None).config(distinct=True)
It seems slightly odd that a build for a single architecture will cause the
whole snap to be considered clean for auto_build_frequency, but I'm not sure
what else to do.
> +
> +@classmethod
> +def makeAutoBuilds(cls, logger=None):
> +"""See `ISnapSet`."""
> +snaps = cls._findStaleSnaps()
> +builds = []
> +for snap in snaps:
> +snap.is_stale = False
> +if logger is not None:
> +logger.debug(
> +"Scheduling builds of snap package %s/%s",
> +snap.owner.name, snap.name)
> +for arch in snap.getAllowedArchitectures():
> +try:
> +build = snap.requestBuild(
> +snap.owner, snap.auto_build_archive, arch,
> +snap.auto_build_pocket)
> +if logger is not None:
> +logger.debug(
> +" - %s: Build requested.", arch.architecturetag)
> +builds.append(build)
> +except Exception as e:
> +if logger is not None:
> +logger.debug(" - %s: %s", arch.architecturetag, e)
That's awfully quiet.
> +return builds
> +
> def detachFromBranch(self, branch):
> """See `ISnapSet`."""
> self.findByBranch(branch).set(
--
https://code.launchpad.net/~cjwatson/launchpad/snap-auto-build-model/+merge/297955
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
