Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/remove-unnecessary-exception-handling into lp:launchpad
Review: Approve Lgtm, +1 -- https://code.launchpad.net/~cjwatson/launchpad/remove-unnecessary-exception-handling/+merge/370616 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-unnecessary-exception-handling into 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/readthedocs-copyright-2019 into lp:launchpad
Review: Approve +1 -- https://code.launchpad.net/~cjwatson/launchpad/readthedocs-copyright-2019/+merge/370480 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/readthedocs-copyright-2019 into 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/s-s-s-eoan into lp:launchpad
Review: Approve +1 -- https://code.launchpad.net/~cjwatson/launchpad/s-s-s-eoan/+merge/369439 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/s-s-s-eoan into 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/upgrade-difftacular into lp:launchpad
Review: Approve +1 -- https://code.launchpad.net/~cjwatson/launchpad/upgrade-difftacular/+merge/368837 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/upgrade-difftacular into 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/remove-lpreview into lp:launchpad
Review: Approve +1 -- https://code.launchpad.net/~cjwatson/launchpad/remove-lpreview/+merge/368835 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/remove-lpreview into 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/bs4-initial into lp:launchpad
Review: Approve Looks good to me. -- https://code.launchpad.net/~cjwatson/launchpad/bs4-initial/+merge/366479 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bs4-initial into 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/celery-4 into lp:launchpad
Review: Approve Looks good to me, celerywise - but I'm not particularly familiar with amqp/amqplib. -- https://code.launchpad.net/~cjwatson/launchpad/celery-4/+merge/364202 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/celery-4 into 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/celery-worker-discard-stderr into lp:launchpad
Review: Approve LGTM. -- https://code.launchpad.net/~cjwatson/launchpad/celery-worker-discard-stderr/+merge/364158 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/celery-worker-discard-stderr into 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/celery-disable-pickle into lp:launchpad
Review: Approve LGTM -- https://code.launchpad.net/~cjwatson/launchpad/celery-disable-pickle/+merge/364045 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/celery-disable-pickle into 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:~twom/launchpad/no-rescan-with-no-jobs into lp:launchpad
Review: Approve LGTM -- https://code.launchpad.net/~twom/launchpad/no-rescan-with-no-jobs/+merge/362479 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~twom/launchpad/no-rescan-with-no-jobs into 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:~twom/launchpad/manually-rescan-link into lp:launchpad
Nice feature, thanks! Just a quick review, with a couple of questions. Diff comments: > > === modified file 'lib/lp/code/model/branch.py' > --- lib/lp/code/model/branch.py 2018-12-10 13:54:34 + > +++ lib/lp/code/model/branch.py 2019-01-23 16:23:25 + > @@ -1295,6 +1295,15 @@ > job.celeryRunOnCommit() > return (self.last_mirrored_id, old_scanned_id) > > +def getLatestScanJob(self): > +from lp.code.model.branchjob import BranchJob, BranchScanJob > +latest_job = IStore(BranchJob).find( > +BranchJob, > +branch=self, > +job_type=BranchScanJob.class_job_type).order_by( Should the ORDER BY be DESC? > +BranchJob.id).first() > +return latest_job > + > def requestMirror(self): > """See `IBranch`.""" > if self.branch_type in (BranchType.REMOTE, BranchType.HOSTED): > > === modified file 'lib/lp/code/model/tests/test_branch.py' > --- lib/lp/code/model/tests/test_branch.py2018-07-09 09:27:06 + > +++ lib/lp/code/model/tests/test_branch.py2019-01-23 16:23:25 + > @@ -3502,6 +3502,19 @@ > getUtility(ILaunchpadCelebrities).commercial_admin): > branch.unscan() > > +def test_getLatestScanJob(self): > +branch = self.factory.makeAnyBranch() > +with person_logged_in(branch.owner): > +branch.unscan(rescan=True) Maybe make sure there are 2+ ScanJobs and check the returned one is actually the latest? > +found = Store.of(branch).find(BranchJob, branch=branch)[0] > +result = branch.getLatestScanJob() > +self.assertEqual(found, result) > + > +def test_getLatestScanJob_no_scans(self): > +branch = self.factory.makeAnyBranch() > +result = branch.getLatestScanJob() > +self.assertIsNone(result) > + > > class TestWebservice(TestCaseWithFactory): > """Tests for the webservice.""" > > === modified file 'lib/lp/code/templates/branch-index.pt' > --- lib/lp/code/templates/branch-index.pt 2017-11-06 09:32:45 + > +++ lib/lp/code/templates/branch-index.pt 2019-01-23 16:23:25 + > @@ -130,18 +130,38 @@ > > > > - > + > + > + > + > + Updating branch... > + > +Launchpad is processing new changes to this branch which will be > +available in a few minutes. Reload to see the changes. > + > + > + > + > + > + > + > > - > -Updating branch... > - > - Launchpad is processing new changes to this branch which will be > - available in a few minutes. Reload to see the changes. > + > +Branch scan failed > + > + Scanning this branch for changes has failed, you can manually > rescan if required. I'd replace the ',' with '.' or ':'. Also, maybe "... you can request a rescan ..."? > + > + > + enctype="multipart/form-data" accept-charset="UTF-8"> > + + name="field.actions.rescan" value="Rescan" /> > + > > > > > > + > > >Recent revisions -- https://code.launchpad.net/~twom/launchpad/manually-rescan-link/+merge/362139 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/git-ssh-url-username into lp:launchpad
Review: Approve Looks good to me. -- https://code.launchpad.net/~cjwatson/launchpad/git-ssh-url-username/+merge/353748 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/git-ssh-url-username into 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/delete-ssh-key-text-type into lp:launchpad
Review: Approve LGTM, thanks. -- https://code.launchpad.net/~cjwatson/launchpad/delete-ssh-key-text-type/+merge/352210 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/delete-ssh-key-text-type into 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/sshkeyadditionerror-msg-format into lp:launchpad
Diff comments:
> === modified file 'lib/lp/registry/interfaces/ssh.py'
> --- lib/lp/registry/interfaces/ssh.py 2018-02-24 09:11:39 +
> +++ lib/lp/registry/interfaces/ssh.py 2018-06-19 21:32:30 +
> @@ -124,4 +124,21 @@
>
> @error_status(httplib.BAD_REQUEST)
> class SSHKeyAdditionError(Exception):
> -"""Raised when the SSH public key is invalid."""
> +"""Raised when the SSH public key is invalid.
> +
> +WARNING: Be careful when changing the message format as
> +SSO uses a regex to recognize it.
> +"""
> +
> +def __init__(self, *args, **kwargs):
> +msg = ""
> +if 'key' in kwargs:
> +key = kwargs.pop('key')
> +msg = "Invalid SSH key data: '%s'" % key
> +if 'kind' in kwargs:
> +kind = kwargs.pop('kind')
> +msg = "Invalid SSH key type: '%s'" % kind
> +if 'exception' in kwargs:
> +exception = kwargs.pop('exception')
> +msg = "%s (%s)" % (msg, str(exception).decode('utf-8', 'ignore'))
That's a lot better. Thanks!
> +super(SSHKeyAdditionError, self).__init__(msg, *args, **kwargs)
--
https://code.launchpad.net/~maxiberta/launchpad/sshkeyadditionerror-msg-format/+merge/348230
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into 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:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad has been updated. Commit message changed to: Fix crash while adding an ssh key with unknown type. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sshkeyadditionerror-msg-format/+merge/348230 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into 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:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad has been updated. Description changed to: Fix encoding crash while adding an ssh key with unknown type. E.g. on an attempt to add this key in SSO /ssh-keys: ssh-rsa asdfasdf comment LP raises an encoding error and crashes. See https://sentry.ols.canonical.com/canonical/sso/issues/669/. Also, add a warning to try to prevent a mismatch with SSO regex matching. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sshkeyadditionerror-msg-format/+merge/348230 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into 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:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad has been updated. Description changed to: Add SSHKeyAdditionError constructor with a warning to try to prevent a mismatch with SSO regex matching. It's already tested in lib/lp/registry/tests/test_ssh.py. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sshkeyadditionerror-msg-format/+merge/348230 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into 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:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad.
Commit message:
Add SSHKeyAdditionError constructor to try to prevent a mismatch with SSO regex
matching.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1777507 in Canonical SSO provider: "UnknownLaunchpadError in what should
be a LaunchpadAPIError causes user-visible oops (adding SSH key)"
https://bugs.launchpad.net/canonical-identity-provider/+bug/1777507
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sshkeyadditionerror-msg-format/+merge/348230
Add SSHKeyAdditionError constructor to try to prevent a mismatch with SSO regex
matching. It's already tested in lib/lp/registry/tests/test_ssh.py.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad.
=== modified file 'lib/lp/registry/interfaces/ssh.py'
--- lib/lp/registry/interfaces/ssh.py 2018-02-24 09:11:39 +
+++ lib/lp/registry/interfaces/ssh.py 2018-06-19 14:47:10 +
@@ -124,4 +124,21 @@
@error_status(httplib.BAD_REQUEST)
class SSHKeyAdditionError(Exception):
-"""Raised when the SSH public key is invalid."""
+"""Raised when the SSH public key is invalid.
+
+WARNING: Be careful when changing the message format as
+SSO uses a regex to recognize it.
+"""
+
+def __init__(self, *args, **kwargs):
+msg = ""
+if 'key' in kwargs:
+key = kwargs.pop('key')
+msg = "Invalid SSH key data: '%s'" % key
+if 'kind' in kwargs:
+kind = kwargs.pop('kind')
+msg = "Invalid SSH key type: '%s'" % kind
+if 'exception' in kwargs:
+exception = kwargs.pop('exception')
+msg = "%s (%s)" % (msg, exception)
+super(SSHKeyAdditionError, self).__init__(msg, *args, **kwargs)
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2018-05-14 10:59:35 +
+++ lib/lp/registry/model/person.py 2018-06-19 14:47:10 +
@@ -4137,8 +4137,7 @@
try:
Key.fromString(sshkey)
except Exception as e:
-raise SSHKeyAdditionError(
-"Invalid SSH key data: '%s' (%s)" % (sshkey, e))
+raise SSHKeyAdditionError(key=sshkey, exception=e)
if send_notification:
person.security_field_changed(
@@ -4171,15 +4170,14 @@
try:
kind, keytext, comment = sshkey.split(' ', 2)
except (ValueError, AttributeError):
-raise SSHKeyAdditionError("Invalid SSH key data: '%s'" % sshkey)
+raise SSHKeyAdditionError(key=sshkey)
if not (kind and keytext and comment):
-raise SSHKeyAdditionError("Invalid SSH key data: '%s'" % sshkey)
+raise SSHKeyAdditionError(key=sshkey)
keytype = SSH_TEXT_TO_KEY_TYPE.get(kind)
if keytype is None:
-raise SSHKeyAdditionError(
-"Invalid SSH key type: '%s'" % kind)
+raise SSHKeyAdditionError(kind=kind)
return keytype, keytext, comment
___
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:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into lp:launchpad has been updated. Description changed to: Add SSHKeyAdditionError constructor to try to prevent a mismatch with SSO regex matching. It's already tested in lib/lp/registry/tests/test_ssh.py. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sshkeyadditionerror-msg-format/+merge/348230 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sshkeyadditionerror-msg-format into 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/new-privacy-policy into lp:launchpad
Review: Approve -- https://code.launchpad.net/~cjwatson/launchpad/new-privacy-policy/+merge/346743 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/new-privacy-policy into 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:~maxiberta/launchpad/sitesearch-drop-google-backend into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/sitesearch-drop-google-backend into lp:launchpad.
Commit message:
Remove the Google search service backend.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-drop-google-backend/+merge/346425
Remove the Google search service backend, leaving all the generic boilerplate
to make adding a new search engine easier next time.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-drop-google-backend into lp:launchpad.
=== modified file 'Makefile'
--- Makefile 2018-04-02 16:40:03 +
+++ Makefile 2018-05-21 21:36:59 +
@@ -64,7 +64,6 @@
bin/bingtestservice \
bin/build-twisted-plugin-cache \
bin/combine-css \
-bin/googletestservice \
bin/harness \
bin/iharness \
bin/ipy \
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2018-03-26 18:59:34 +
+++ configs/development/launchpad-lazr.conf 2018-05-21 21:36:59 +
@@ -78,14 +78,6 @@
oops_prefix: X
error_dir: /var/tmp/lperr
-[google]
-# Development and the testrunner should use the stub service by default.
-site: http://launchpad.dev:8092/cse
-client_id: ABCDEF2323
-
-[google_test_service]
-launch: True
-
[bing]
# Development and the testrunner should use the stub service by default.
site: http://launchpad.dev:8093/bingcustomsearch/v7.0/search
=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf 2018-03-16 21:04:45 +
+++ configs/testrunner-appserver/launchpad-lazr.conf 2018-05-21 21:36:59 +
@@ -11,9 +11,6 @@
[codeimport]
macaroon_secret_key: dev-macaroon-secret
-[google_test_service]
-launch: False
-
[bing_test_service]
launch: False
=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2018-03-16 21:04:45 +
+++ configs/testrunner/launchpad-lazr.conf 2018-05-21 21:36:59 +
@@ -88,9 +88,6 @@
source_only: True
root: /tmp/gina_test_archive
-[google]
-site: http://launchpad.dev:8092/cse
-
[bing]
site: http://launchpad.dev:8093/bingcustomsearch/v7.0/search
@@ -108,8 +105,6 @@
max_attachment_size: 1024
geoip_database: /usr/share/GeoIP/GeoLiteCity.dat
logparser_max_parsed_lines: 10
-# We use the stub Google Service here which maps URL fragment to
-# to static content
homepage_recent_posts_feed: http://launchpad.dev:8092/blog-feed
openid_canonical_root: http://testopenid.dev/
openid_provider_root: http://testopenid.dev/
=== modified file 'lib/lp/app/browser/doc/launchpad-search-pages.txt'
--- lib/lp/app/browser/doc/launchpad-search-pages.txt 2018-04-13 21:57:12 +
+++ lib/lp/app/browser/doc/launchpad-search-pages.txt 2018-05-21 21:36:59 +
@@ -406,7 +406,7 @@
Page searches
-
-The view uses the GoogleSearchService to locate pages that match the
+The view uses the Search Service to locate pages that match the
search terms.
>>> search_view = getSearchView(
@@ -418,23 +418,23 @@
>>> search_view.pages
<...SiteSearchBatchNavigator ...>
-The GoogleSearchService may not be available due to connectivity problems.
+The Search Service may not be available due to connectivity problems.
The view's has_page_service attribute reports when the search was performed
-with Google page matches.
+with the Search Service page matches.
>>> search_view.has_page_service
True
The batch navigation heading is created by the view. The heading
property returns a 2-tuple of singular and plural heading. There
-is a heading when there are only Google page matches...
+is a heading when there are only Search Service page matches...
>>> search_view.has_exact_matches
False
>>> search_view.batch_heading
(u'page matching "bug"', u'pages matching "bug"')
-...and a heading for when there are exact matches and Google page
+...and a heading for when there are exact matches and Search Service page
matches.
>>> search_view = getSearchView(
@@ -518,14 +518,11 @@
>>> page.summary
u'...Launchpad\u2019s ...bug... tracker allows collaboration...'
-See `google-searchservice.txt` for more information about the
-GoogleSearchService and PageMatch objects.
-
No page matches
---
-When an empty PageMatches object is returned by the GoogleSearchService to
+When an empty PageMatches object is returned by the Search Service to
the view, there are no matches to show.
>>> search_view = getSearchView(form={'field.text': 'no-meaningful'})
@@ -549,11 +546,11 @@
True
-Bad Google response handling
+Bad site search response handling
Connectivity problems can cause mis
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-store-release-refresh into lp:launchpad
Review: Approve LGTM. -- https://code.launchpad.net/~cjwatson/launchpad/snap-store-release-refresh/+merge/344997 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-store-release-refresh into 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:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad has been updated. Description changed to: Add sitesearch tests based on doctests. - sitesearch/doc/google-searchservice.txt => sitesearch/tests/test_google.py - sitesearch/doc/bing-searchservice.txt => sitesearch/tests/test_bing.py Add PageMatch and PageMatches unittests based on doctests. - sitesearch/doc/*-searchservice.txt => sitesearch/tests/test_pagematch.py Drop all sitesearch doctests (lib/lp/sitesearch/doc). For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-2/+merge/343236 -- 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:~maxiberta/launchpad/app-browser-dedup-sitesearch-doctests into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/app-browser-dedup-sitesearch-doctests into lp:launchpad.
Commit message:
Deduplicate lp.app.browser sitesearch doctests.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/app-browser-dedup-sitesearch-doctests/+merge/343250
Deduplicate lp.app.browser sitesearch doctests. In particular,
launchpad-search-pages-bing.txt and launchpad-search-pages-google.txt were
merged into one single file, which is tested with all search backends.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/app-browser-dedup-sitesearch-doctests into lp:launchpad.
=== removed file 'lib/lp/app/browser/doc/launchpad-search-pages-bing.txt'
--- lib/lp/app/browser/doc/launchpad-search-pages-bing.txt 2018-04-10 18:17:07 +
+++ lib/lp/app/browser/doc/launchpad-search-pages-bing.txt 1970-01-01 00:00:00 +
@@ -1,726 +0,0 @@
-Launchpad search page
-=
-
-Users can search for Launchpad objects and pages from the search form
-located on all pages. The search is performed and displayed by the
-LaunchpadSearchView.
-
->>> from zope.component import getMultiAdapter, getUtility
->>> from lp.services.webapp.interfaces import ILaunchpadRoot
->>> from lp.services.webapp.servers import LaunchpadTestRequest
-
->>> root = getUtility(ILaunchpadRoot)
->>> request = LaunchpadTestRequest()
->>> search_view = getMultiAdapter((root, request), name="+search")
->>> search_view.initialize()
->>> search_view
-<SimpleViewClass from .../templates/launchpad-search.pt ...>
-
-
-Page title and heading
---
-
-The page title and heading suggest to the user to search launchpad
-when there is no search text.
-
->>> print search_view.text
-None
->>> search_view.page_title
-'Search Launchpad'
->>> search_view.page_heading
-'Search Launchpad'
-
-When text is not None, the title indicates what was searched.
-
->>> def getSearchView(form):
-... search_param_list = []
-... for name in sorted(form):
-... value = form[name]
-... search_param_list.append('%s=%s' % (name, value))
-... query_string = '&'.join(search_param_list)
-... request = LaunchpadTestRequest(
-... SERVER_URL='https://launchpad.dev/+search',
-... QUERY_STRING=query_string, form=form, PATH_INFO='/+search')
-... search_view = getMultiAdapter((root, request), name="+search")
-... search_view.initialize()
-... return search_view
-
->>> search_view = getSearchView(
-... form={'field.text': 'albatross'})
-
->>> search_view.text
-u'albatross'
->>> search_view.page_title
-u'Pages matching "albatross" in Launchpad'
->>> search_view.page_heading
-u'Pages matching "albatross" in Launchpad'
-
-
-No matches
---
-
-There were no matches for 'albatross'.
-
->>> search_view.has_matches
-False
-
-When search text is not submitted there are no matches. Search text is
-required to perform a search. Note that field.actions.search is not a
-required param to call the Search Action. The view always calls the
-search action.
-
->>> search_view = getSearchView(form={})
-
->>> print search_view.text
-None
->>> search_view.has_matches
-False
-
-
-Bug and Question Searches
--
-
-When a numeric token can be extracted from the submitted search text,
-the view tries to match a bug and question. Bugs and questions are
-matched by their id.
-
->>> search_view = getSearchView(
-... form={'field.text': '5'})
->>> search_view._getNumericToken(search_view.text)
-u'5'
->>> search_view.has_matches
-True
->>> search_view.bug.title
-u'Firefox install instructions should be complete'
->>> search_view.question.title
-u'Installation failed'
-
-Bugs and questions are matched independent of each other. The number
-extracted may only match one kind of object. For example, there are
-more bugs than questions.
-
->>> search_view = getSearchView(
-... form={'field.text': '15'})
->>> search_view._getNumericToken(search_view.text)
-u'15'
->>> search_view.has_matches
-True
->>> search_view.bug.title
-u'Nonsensical bugs are useless'
->>> print search_view.question
-None
-
-Private bugs are not matched if the user does not have permission to
-see them. For example, Sample Person can see a private bug that they
-created beca
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/app-stories-dedup-sitesearch-doctests into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/app-stories-dedup-sitesearch-doctests into lp:launchpad.
Commit message:
Run lp.app.stories.launchpad_search.site-search.txt on all search backends.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/app-stories-dedup-sitesearch-doctests/+merge/343244
Run lp.app.stories.launchpad_search.site-search.txt on all search backends (was
lp.app.stories.launchpad_root.site-search.txt).
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/app-stories-dedup-sitesearch-doctests into lp:launchpad.
=== added directory 'lib/lp/app/stories/launchpad-search'
=== renamed file 'lib/lp/app/stories/launchpad-root/site-search.txt' => 'lib/lp/app/stories/launchpad-search/site-search.txt'
=== modified file 'lib/lp/app/tests/test_doc.py'
--- lib/lp/app/tests/test_doc.py 2011-12-30 09:16:36 +
+++ lib/lp/app/tests/test_doc.py 2018-04-13 21:01:49 +
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""
@@ -7,8 +7,13 @@
import os
+from lp.services.features.testing import FeatureFixture
from lp.services.testing import build_test_suite
from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.pages import (
+PageTestSuite,
+setUpGlobs,
+)
from lp.testing.systemdocs import (
LayeredDocFileSuite,
setUp,
@@ -17,6 +22,29 @@
here = os.path.dirname(os.path.realpath(__file__))
+bing_flag = FeatureFixture({'sitesearch.engine.name': 'bing'})
+google_flag = FeatureFixture({'sitesearch.engine.name': 'google'})
+
+
+def setUp_bing(test):
+setUpGlobs(test)
+bing_flag.setUp()
+
+
+def setUp_google(test):
+setUpGlobs(test)
+google_flag.setUp()
+
+
+def tearDown_bing(test):
+bing_flag.cleanUp()
+tearDown(test)
+
+
+def tearDown_google(test):
+google_flag.cleanUp()
+tearDown(test)
+
special = {
'tales.txt': LayeredDocFileSuite(
@@ -27,6 +55,21 @@
'menus.txt': LayeredDocFileSuite(
'../doc/menus.txt', layer=None,
),
+'stories/launchpad-search(Bing)': PageTestSuite(
+'../stories/launchpad-search/',
+id_extensions=['site-search.txt(Bing)'],
+setUp=setUp_bing, tearDown=tearDown_bing,
+),
+'stories/launchpad-search(Google)': PageTestSuite(
+'../stories/launchpad-search/',
+id_extensions=['site-search.txt(Google)'],
+setUp=setUp_google, tearDown=tearDown_google,
+),
+# Run these doctests again with the default search engine.
+'../stories/launchpad-search': PageTestSuite(
+'../stories/launchpad-search/',
+setUp=setUpGlobs, tearDown=tearDown,
+),
}
=== modified file 'lib/lp/services/testing/__init__.py'
--- lib/lp/services/testing/__init__.py 2011-12-30 07:08:24 +
+++ lib/lp/services/testing/__init__.py 2018-04-13 21:01:49 +
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""
@@ -98,11 +98,12 @@
if not os.path.isdir(full_story_dir):
continue
story_path = os.path.join(stories_dir, story_dir)
+if story_path in special_tests:
+continue
suite.addTest(PageTestSuite(story_path, package))
# Add the special doctests.
-for key in sorted(special_tests):
-special_suite = special_tests[key]
+for key, special_suite in sorted(special_tests.items()):
suite.addTest(special_suite)
tests_path = os.path.join(os.path.pardir, 'doc')
=== modified file 'lib/lp/testing/pages.py'
--- lib/lp/testing/pages.py 2017-10-21 18:14:14 +
+++ lib/lp/testing/pages.py 2018-04-13 21:01:49 +
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Testing infrastructure for page tests."""
@@ -845,7 +845,7 @@
# but does follow the convention of the other doctest related *Suite()
# functions.
-def PageTestSuite(storydir, package=None, setUp=setUpGlobs):
+def PageTestSuite(storydir, package=None, setUp=setUpGlobs, **kw):
"""Create a suite of page tests for files found in storydir.
:param storydir: the directory containing the page tests.
@@ -873,5 +873,5 @@
suite.addTest(LayeredDocFileSuite(
paths=paths,
package=package, checke
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/sitesearch-dedup-tests into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/sitesearch-dedup-tests into lp:launchpad.
Commit message:
Deduplicate sitesearch testservices testing code.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-dedup-tests/+merge/343241
Use test scenarios to deduplicate sitesearch testservices testing code.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-dedup-tests into lp:launchpad.
=== removed file 'lib/lp/services/sitesearch/tests/test_bingservice.py'
--- lib/lp/services/sitesearch/tests/test_bingservice.py 2018-03-26 20:05:20 +
+++ lib/lp/services/sitesearch/tests/test_bingservice.py 1970-01-01 00:00:00 +
@@ -1,38 +0,0 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""
-Unit tests for the Bing test service stub.
-"""
-
-__metaclass__ = type
-
-
-import os
-import unittest
-
-from lp.services.osutils import process_exists
-from lp.services.pidfile import pidfile_path
-from lp.services.sitesearch import bingtestservice
-
-
-class TestServiceUtilities(unittest.TestCase):
-"""Test the service's supporting functions."""
-
-def test_stale_pid_file_cleanup(self):
-"""The service should be able to clean up invalid PID files."""
-bogus_pid = 999
-self.assertFalse(
-process_exists(bogus_pid),
-"There is already a process with PID '%d'." % bogus_pid)
-
-# Create a stale/bogus PID file.
-filepath = pidfile_path(bingtestservice.service_name)
-with file(filepath, 'w') as pidfile:
-pidfile.write(str(bogus_pid))
-
-# The PID clean-up code should silently remove the file and return.
-bingtestservice.kill_running_process()
-self.assertFalse(
-os.path.exists(filepath),
-"The PID file '%s' should have been deleted." % filepath)
=== renamed file 'lib/lp/services/sitesearch/tests/test_googleservice.py' => 'lib/lp/services/sitesearch/tests/test_testservices.py'
--- lib/lp/services/sitesearch/tests/test_googleservice.py 2018-03-27 17:43:27 +
+++ lib/lp/services/sitesearch/tests/test_testservices.py 2018-04-13 20:37:40 +
@@ -1,9 +1,9 @@
# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-"""
-Unit tests for the Google test service stub.
-"""
+"""Unit tests for sitesearch test service stubs."""
+
+from __future__ import absolute_import, print_function, unicode_literals
__metaclass__ = type
@@ -11,14 +11,28 @@
import os
import unittest
+from testscenarios import WithScenarios
+
from lp.services.osutils import process_exists
from lp.services.pidfile import pidfile_path
-from lp.services.sitesearch import googletestservice
-
-
-class TestServiceUtilities(unittest.TestCase):
+from lp.services.sitesearch import (
+bingtestservice,
+googletestservice,
+)
+
+
+class TestServiceUtilities(WithScenarios, unittest.TestCase):
"""Test the service's supporting functions."""
+scenarios = [
+("Bing", {
+"testservice": bingtestservice,
+}),
+("Google", {
+"testservice": googletestservice,
+}),
+]
+
def test_stale_pid_file_cleanup(self):
"""The service should be able to clean up invalid PID files."""
bogus_pid = 999
@@ -27,13 +41,12 @@
"There is already a process with PID '%d'." % bogus_pid)
# Create a stale/bogus PID file.
-filepath = pidfile_path(googletestservice.service_name)
-pidfile = file(filepath, 'w')
-pidfile.write(str(bogus_pid))
-pidfile.close()
+filepath = pidfile_path(self.testservice.service_name)
+with file(filepath, 'w') as pidfile:
+pidfile.write(str(bogus_pid))
# The PID clean-up code should silently remove the file and return.
-googletestservice.kill_running_process()
+self.testservice.kill_running_process()
self.assertFalse(
os.path.exists(filepath),
"The PID file '%s' should have been deleted." % filepath)
___
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:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad has been updated. Description changed to: Add sitesearch tests based on doctests. - sitesearch/doc/google-searchservice.txt => sitesearch/tests/test_google.py - sitesearch/doc/bing-searchservice.txt => sitesearch/tests/test_bing.py Add PageMatch and PageMatches unittests based on doctests. - sitesearch/doc/*-searchservice.txt => sitesearch/tests/test_pagematch.py For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-2/+merge/343236 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sitesearch-cleanup-2 into 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:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad has been updated. Commit message changed to: Add sitesearch tests based on doctests. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-2/+merge/343236 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sitesearch-cleanup-2 into 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:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad with
lp:~maxiberta/launchpad/sitesearch-cleanup-1 as a prerequisite.
Commit message:
Assorted sitesearch fixes and improvements (part 2).
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-2/+merge/343236
Assorted sitesearch fixes and improvements (part 2).
- Add sitesearch unittests based on doctests.
- sitesearch/doc/google-searchservice.txt => sitesearch/tests/test_google.py
- sitesearch/doc/bing-searchservice.txt => sitesearch/tests/test_bing.py
- Add PageMatch and PageMatches unittests based on doctests.
- sitesearch/doc/*-searchservice.txt => sitesearch/tests/test_pagematch.py
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad.
=== modified file 'lib/lp/services/sitesearch/tests/test_bing.py'
--- lib/lp/services/sitesearch/tests/test_bing.py 2018-04-12 19:42:23 +
+++ lib/lp/services/sitesearch/tests/test_bing.py 2018-04-13 19:42:12 +
@@ -1,4 +1,4 @@
-# Copyright 2011-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the bing search service."""
@@ -7,31 +7,177 @@
__metaclass__ = type
+import json
+from os import path
+
from fixtures import MockPatch
from requests.exceptions import (
ConnectionError,
HTTPError,
)
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+HasLength,
+MatchesStructure,
+)
+from lp.services.config import config
from lp.services.sitesearch import BingSearchService
from lp.services.sitesearch.interfaces import SiteSearchResponseError
from lp.services.timeout import TimeoutError
from lp.testing import TestCase
-from lp.testing.layers import (
-BingLaunchpadFunctionalLayer,
-LaunchpadFunctionalLayer,
-)
+from lp.testing.layers import BingLaunchpadFunctionalLayer
class TestBingSearchService(TestCase):
"""Test BingSearchService."""
-layer = LaunchpadFunctionalLayer
-
def setUp(self):
super(TestBingSearchService, self).setUp()
self.search_service = BingSearchService()
+self.base_path = path.normpath(
+path.join(path.dirname(__file__), 'data'))
+
+def test_configuration(self):
+self.assertEqual(config.bing.site, self.search_service.site)
+self.assertEqual(
+config.bing.subscription_key, self.search_service.subscription_key)
+self.assertEqual(
+config.bing.custom_config_id, self.search_service.custom_config_id)
+
+def test_create_search_url(self):
+self.assertEndsWith(
+self.search_service.create_search_url(terms='svg +bugs'),
+'=0=svg+%2Bbugs')
+
+def test_create_search_url_escapes_unicode_chars(self):
+self.assertEndsWith(
+self.search_service.create_search_url('Carlo Perell\xf3 Mar\xedn'),
+'=0=Carlo+Perell%C3%B3+Mar%C3%ADn')
+
+def test_create_search_url_with_offset(self):
+self.assertEndsWith(
+self.search_service.create_search_url(terms='svg +bugs', start=20),
+'=20=svg+%2Bbugs')
+
+def test_create_search_url_empty_terms(self):
+e = self.assertRaises(
+ValueError, self.search_service.create_search_url, '')
+self.assertEqual("Missing value for parameter 'q'.", str(e))
+
+def test_create_search_url_null_terms(self):
+e = self.assertRaises(
+ValueError, self.search_service.create_search_url, None)
+self.assertEqual("Missing value for parameter 'q'.", str(e))
+
+def test_create_search_url_requires_start(self):
+e = self.assertRaises(
+ValueError, self.search_service.create_search_url, 'bugs', 'true')
+self.assertEqual("Value for parameter 'offset' is not an int.", str(e))
+
+def test_parse_search_response_invalid_total(self):
+"""The PageMatches's total attribute comes from the
+`webPages.totalEstimatedMatches` JSON element.
+When it cannot be found and the value cast to an int,
+an error is raised. If Bing were to redefine the meaning of the
+element to use a '~' to indicate an approximate total, an error would
+be raised.
+"""
+file_name = path.join(
+self.base_path, 'bingsearchservice-incompatible-matches.json')
+with open(file_name, 'r') as response_file:
+response = response_file.read()
+assert (
+json.loads(response)['webPages']['totalEstimatedMatches'] == '~25'
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup-2 into lp:launchpad has been updated. Status: Work in progress => Superseded For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-2/+merge/343235 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sitesearch-cleanup-2 into 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:~maxiberta/launchpad/sitesearch-cleanup-1 into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/sitesearch-cleanup-1 into lp:launchpad.
Commit message:
Assorted sitesearch fixes and improvements.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-1/+merge/343131
Assorted sitesearch fixes and improvements.
- Rename BingSearchService._parse_bing_response() and
GoogleSearchService._parse_google_search_protocol() as
._parse_search_response().
- Move config.{bing,google}.url_rewrite_exceptions into config.sitesearch.
- Fix wrong content type of bingtestservice responses.
- Make googletestservice.py script executable.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-cleanup-1 into lp:launchpad.
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2018-03-26 18:59:34 +
+++ lib/lp/services/config/schema-lazr.conf 2018-04-12 21:30:28 +
@@ -788,6 +788,7 @@
# datatype: string, a url to a host
site: http://www.google.com/cse
+[sitesearch]
# url_rewrite_exceptions is a list of launchpad.net domains that must
# not be rewritten.
# datatype: string of space separated domains
@@ -824,12 +825,6 @@
# datatype: string
custom_config_id:
-# url_rewrite_exceptions is a list of launchpad.net domains that must
-# not be rewritten.
-# datatype: string of space separated domains
-# Example: help.launchpad.net login.launchpad.net
-url_rewrite_exceptions: help.launchpad.net
-
[gpghandler]
# Should we allow uploading keys to the keyserver?
# datatype: boolean
=== modified file 'lib/lp/services/sitesearch/__init__.py'
--- lib/lp/services/sitesearch/__init__.py 2018-04-10 16:15:17 +
+++ lib/lp/services/sitesearch/__init__.py 2018-04-12 21:30:28 +
@@ -53,9 +53,9 @@
def url_rewrite_exceptions(self):
"""A list of launchpad.net URLs that must not be rewritten.
-Configured in config.google.url_rewrite_exceptions.
+Configured in config.sitesearch.url_rewrite_exceptions.
"""
-return config.google.url_rewrite_exceptions.split()
+return config.sitesearch.url_rewrite_exceptions.split()
@property
def url_rewrite_scheme(self):
@@ -221,7 +221,7 @@
"The response errored: %s" % str(error))
finally:
action.finish()
-page_matches = self._parse_google_search_protocol(response.content)
+page_matches = self._parse_search_response(response.content)
return page_matches
def _checkParameter(self, name, value, is_int=False):
@@ -272,7 +272,7 @@
"""
return self._getElementsByAttributeValue(doc, path, name, value)[0]
-def _parse_google_search_protocol(self, gsp_xml):
+def _parse_search_response(self, gsp_xml):
"""Return a `PageMatches` object.
:param gsp_xml: A string that should be Google Search Protocol
@@ -397,7 +397,7 @@
"The response errored: %s" % str(error))
finally:
action.finish()
-page_matches = self._parse_bing_response(response.content, start)
+page_matches = self._parse_search_response(response.content, start)
return page_matches
def _checkParameter(self, name, value, is_int=False):
@@ -430,7 +430,7 @@
'Ocp-Apim-Subscription-Key': self.subscription_key,
}
-def _parse_bing_response(self, bing_json, start=0):
+def _parse_search_response(self, bing_json, start=0):
"""Return a `PageMatches` object.
:param bing_json: A string containing Bing Custom Search API v7 JSON.
=== modified file 'lib/lp/services/sitesearch/bingtestservice.py'
--- lib/lp/services/sitesearch/bingtestservice.py 2018-03-28 21:28:12 +
+++ lib/lp/services/sitesearch/bingtestservice.py 2018-04-12 21:30:28 +
@@ -8,6 +8,10 @@
when given certain user-configurable URLs.
"""
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
import logging
import os
@@ -27,7 +31,7 @@
class BingRequestHandler(testservice.RequestHandler):
-default_content_type = 'text/xml; charset=UTF-8'
+default_content_type = 'application/json; charset=UTF-8'
log = log
mapfile = config.bing_test_service.mapfile
content_dir = config.bing_test_service.canned_response_directory
=== modified file 'lib/lp/services/sitesearch/doc/bing-searchservice.txt'
--- lib/lp/services/sitesearch/doc/bing-searchservice.txt 2018-04-10 16:15:17 +
+++ lib/lp/services/sitesearch/doc/bing-searchservice.txt 2018-04-12 21:30:28 +
@@ -72,7 +72,7 @@
of returned results.
The first search for 'bugs' returned a subset of items in the
-ISearchResult. There are 25 total items, but the results
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad
Review: Resubmit I'm splitting this into more manageable MP's. The first one: https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup-1/+merge/343131. -- https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup/+merge/342472 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sitesearch-cleanup into 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:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup/+merge/342472 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/sitesearch-cleanup into 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/bing-xss into lp:launchpad
Thanks! -- https://code.launchpad.net/~cjwatson/launchpad/bing-xss/+merge/342950 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bing-xss into 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/bing-xss into lp:launchpad
Review: Approve LGTM. An extra test in sitesearch/tests/test_bing.py would be great though. -- https://code.launchpad.net/~cjwatson/launchpad/bing-xss/+merge/342950 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/bing-xss into 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:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup into
lp:launchpad has been updated.
Description changed to:
Sitesearch code cleanup:
- Add unittests based on sitesearch doctests.
- These doctests are still tested; unsure of removing as they're still useful
documentation.
- Add tests for PageMatch and PageMatches.
- Use test scenarios to deduplicate sitesearch testservices testing code.
- Deduplicate lp.app.browser search doctests.
- Run lp.app.stories.launchpad_search.site-search.txt on all search backends
(was launchpad_root.site-search.txt).
- Fix permissions on googletestservice.py script.
- Fix content type of bingtestservice responses.
- Move config.{bing,google}.url_rewrite_exceptions into config.sitesearch.
- Update python sources following the standard template.
- Make Bing the default (see
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-default-bing/+merge/342473).
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup/+merge/342472
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-cleanup into 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:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup into
lp:launchpad has been updated.
Description changed to:
Sitesearch code cleanup:
- Add unittests based on sitesearch doctests.
- These doctests are still tested; unsure of removing as they're still useful
documentation.
- Add tests for PageMatch and PageMatches.
- Use test scenarios to deduplicate sitesearch testservices testing code.
- Deduplicate lp.app.browser search doctests.
- Run lp.app.stories.launchpad_search.site-search.txt on all search backends
(was launchpad_root.site-search.txt).
- Fix permissions on googletestservice.py script.
- Fix content type of bingtestservice responses.
- Move config.{bing,google}.url_rewrite_exceptions into config.sitesearch.
- Update python sources following the standard template.
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup/+merge/342472
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-cleanup into 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:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/sitesearch-cleanup into
lp:launchpad has been updated.
Description changed to:
Sitesearch code cleanup:
- Add unittests based on sitesearch doctests.
- These doctests are still tested; unsure of removing as they're still useful
documentation.
- Add tests for PageMatch and PageMatches.
- Use test scenarios to deduplicate sitesearch testservices testing code.
- Fix permissions on googletestservice.py script.
- Fix content type of bingtestservice responses.
- Move config.{bing,google}.url_rewrite_exceptions into config.sitesearch.
- Update python sources following the standard template.
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup/+merge/342472
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-cleanup into 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:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad.
Commit message:
Sitesearch code cleanup.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-cleanup/+merge/342472
Sitesearch code cleanup:
- Add unittests based on sitesearch doctests.
- These doctests are still tested; unsure of removing as they're still useful
documentation.
- Add tests for PageMatch and PageMatches.
- Use test scenarios to deduplicate sitesearch testservices testing code.
- Fix permissions on googletestservice.py script.
- Move config.{bing,google}.url_rewrite_exceptions into config.sitesearch.
- Update python sources following the standard template.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-cleanup into lp:launchpad.
=== modified file 'lib/lp/app/browser/doc/launchpad-search-pages-bing.txt'
--- lib/lp/app/browser/doc/launchpad-search-pages-bing.txt 2018-03-28 19:31:02 +
+++ lib/lp/app/browser/doc/launchpad-search-pages-bing.txt 2018-03-30 23:12:56 +
@@ -508,16 +508,14 @@
are used for making links to the pages. The summary contains markup
showing the matching terms in context of the page text.
->>> print range(20)
-[0, 1, ..., 18, 19]
>>> page = pages[0]
>>> page
<...PageMatch ...>
>>> page.title
u'Bugs - Launchpad Help'
>>> page.url
-'https://help.launchpad.net/Bugs'
->>> page.summary # doctest: +ELLIPSIS
+'http://bugs.launchpad.dev/ubuntu/hoary/+bug/2'
+>>> page.summary
u"Launchpad Help > Bugs . Use Launchpad's bug tracker for your project..."
See `google-searchservice.txt` for more information about the
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2018-03-26 18:59:34 +
+++ lib/lp/services/config/schema-lazr.conf 2018-03-30 23:12:56 +
@@ -788,6 +788,7 @@
# datatype: string, a url to a host
site: http://www.google.com/cse
+[sitesearch]
# url_rewrite_exceptions is a list of launchpad.net domains that must
# not be rewritten.
# datatype: string of space separated domains
@@ -824,12 +825,6 @@
# datatype: string
custom_config_id:
-# url_rewrite_exceptions is a list of launchpad.net domains that must
-# not be rewritten.
-# datatype: string of space separated domains
-# Example: help.launchpad.net login.launchpad.net
-url_rewrite_exceptions: help.launchpad.net
-
[gpghandler]
# Should we allow uploading keys to the keyserver?
# datatype: boolean
=== modified file 'lib/lp/services/sitesearch/__init__.py'
--- lib/lp/services/sitesearch/__init__.py 2018-03-27 17:43:27 +
+++ lib/lp/services/sitesearch/__init__.py 2018-03-30 23:12:56 +
@@ -52,9 +52,9 @@
def url_rewrite_exceptions(self):
"""A list of launchpad.net URLs that must not be rewritten.
-Configured in config.google.url_rewrite_exceptions.
+Configured in config.sitesearch.url_rewrite_exceptions.
"""
-return config.google.url_rewrite_exceptions.split()
+return config.sitesearch.url_rewrite_exceptions.split()
@property
def url_rewrite_scheme(self):
@@ -220,7 +220,7 @@
"The response errored: %s" % str(error))
finally:
action.finish()
-page_matches = self._parse_google_search_protocol(response.content)
+page_matches = self._parse_search_response(response.content)
return page_matches
def _checkParameter(self, name, value, is_int=False):
@@ -271,7 +271,7 @@
"""
return self._getElementsByAttributeValue(doc, path, name, value)[0]
-def _parse_google_search_protocol(self, gsp_xml):
+def _parse_search_response(self, gsp_xml):
"""Return a `PageMatches` object.
:param gsp_xml: A string that should be Google Search Protocol
@@ -396,7 +396,7 @@
"The response errored: %s" % str(error))
finally:
action.finish()
-page_matches = self._parse_bing_response(response.content, start)
+page_matches = self._parse_search_response(response.content, start)
return page_matches
def _checkParameter(self, name, value, is_int=False):
@@ -429,7 +429,7 @@
'Ocp-Apim-Subscription-Key': self.subscription_key,
}
-def _parse_bing_response(self, bing_json, start=0):
+def _parse_search_response(self, bing_json, start=0):
"""Return a `PageMatches` object.
:param bing_json: A string containing Bing Custom Search API v7 JSON.
=== modified file 'lib/lp/services/sitesearch/bingtestservice.py'
--- lib/lp/services/sitesearch/bingtestservice.py 2018-03-2
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/sitesearch-default-bing into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/sitesearch-default-bing into lp:launchpad.
Commit message:
Make Bing the default site search engine.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/sitesearch-default-bing/+merge/342473
Make Bing the default site search engine, now that it's been successfully
deployed.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/sitesearch-default-bing into lp:launchpad.
=== modified file 'lib/lp/app/browser/root.py'
--- lib/lp/app/browser/root.py 2018-03-28 12:52:32 +
+++ lib/lp/app/browser/root.py 2018-03-30 23:13:48 +
@@ -517,8 +517,8 @@
if query_terms in [None, '']:
return None
search_engine = getFeatureFlag("sitesearch.engine.name")
-# Default to the Google search engine.
-search_engine = search_engine or "google"
+# Default to the Bing search engine.
+search_engine = search_engine or "bing"
site_search = getUtility(ISearchService, name=search_engine)
try:
page_matches = site_search.search(
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2018-03-28 12:52:32 +
+++ lib/lp/services/features/flags.py 2018-03-30 23:13:48 +
@@ -237,7 +237,7 @@
('sitesearch.engine.name',
'space delimited',
'Name of the site search engine backend ("google" or "bing").',
- 'google',
+ 'bing',
'Site search engine',
''),
])
___
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:~maxiberta/launchpad/bing-search into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/bing-search into lp:launchpad has been updated. Description changed to: Add basic Bing Custom Search support. Should be pretty unobtrusive, while adding a basic site search implementation around Bing Custom Search (shamelessly copied from the Google site search implementation). Might need some more testing; and could use some code deduplication. Deployment notes: - New config options: "bing.subscription_key" and "bing.custom_config_id". - New feature flag "sitesearch.engine.name" should be set to "bing" (defaults to "google"). For more details, see: https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 -- 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:~maxiberta/launchpad/bing-search into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/bing-search into lp:launchpad.
Commit message:
Add basic Bing Custom Search site search support.
Requested reviews:
Colin Watson (cjwatson)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549
Add basic Bing Custom Search support.
Should be pretty unobtrusive, while adding a basic site search implementation
around Bing Custom Search (shamelessly copied from the Google site search
implementation). Might need some more testing; and could use some code
deduplication.
--
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'Makefile'
--- Makefile 2018-02-01 20:56:23 +
+++ Makefile 2018-03-28 21:29:05 +
@@ -61,6 +61,7 @@
# NB: It's important PIP_BIN only mentions things genuinely produced by pip.
PIP_BIN = \
$(PY) \
+bin/bingtestservice \
bin/build-twisted-plugin-cache \
bin/combine-css \
bin/googletestservice \
@@ -284,7 +285,7 @@
bin/test -f $(TESTFLAGS) $(TESTOPTS)
run: build inplace stop
- bin/run -r librarian,google-webservice,memcached,rabbitmq,txlongpoll \
+ bin/run -r librarian,bing-webservice,google-webservice,memcached,rabbitmq,txlongpoll \
-i $(LPCONFIG)
run-testapp: LPCONFIG=testrunner-appserver
@@ -297,12 +298,12 @@
start-gdb: build inplace stop support_files run.gdb
nohup gdb -x run.gdb --args bin/run -i $(LPCONFIG) \
- -r librarian,google-webservice
+ -r librarian,bing-webservice,google-webservice
> ${LPCONFIG}-nohup.out 2>&1 &
run_all: build inplace stop
bin/run \
- -r librarian,sftp,forker,mailman,codebrowse,google-webservice,\
+ -r librarian,sftp,forker,mailman,codebrowse,bing-webservice,google-webservice,\
memcached,rabbitmq,txlongpoll -i $(LPCONFIG)
run_codebrowse: compile
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2018-02-02 15:29:38 +
+++ configs/development/launchpad-lazr.conf 2018-03-28 21:29:05 +
@@ -79,13 +79,22 @@
error_dir: /var/tmp/lperr
[google]
-# Development and the testrunner should use the stub service be default.
+# Development and the testrunner should use the stub service by default.
site: http://launchpad.dev:8092/cse
client_id: ABCDEF2323
[google_test_service]
launch: True
+[bing]
+# Development and the testrunner should use the stub service by default.
+site: http://launchpad.dev:8093/bingcustomsearch/v7.0/search
+subscription_key: abcdef01234567890abcdef012345678
+custom_config_id: 1234567890
+
+[bing_test_service]
+launch: True
+
[gpghandler]
host: keyserver.launchpad.dev
public_host: keyserver.launchpad.dev
=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf 2016-10-11 15:28:25 +
+++ configs/testrunner-appserver/launchpad-lazr.conf 2018-03-28 21:29:05 +
@@ -14,6 +14,9 @@
[google_test_service]
launch: False
+[bing_test_service]
+launch: False
+
[launchpad]
openid_provider_root: http://testopenid.dev:8085/
=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2018-01-26 22:18:38 +
+++ configs/testrunner/launchpad-lazr.conf 2018-03-28 21:29:05 +
@@ -91,6 +91,9 @@
[google]
site: http://launchpad.dev:8092/cse
+[bing]
+site: http://launchpad.dev:8093/bingcustomsearch/v7.0/search
+
[gpghandler]
upload_keys: True
host: localhost
=== added directory 'lib/lp/app/browser/doc'
=== added directory 'lib/lp/app/browser/doc.moved'
=== renamed file 'lib/lp/app/browser/tests/base-layout.txt' => 'lib/lp/app/browser/doc/base-layout.txt'
--- lib/lp/app/browser/tests/base-layout.txt 2016-03-14 00:45:42 +
+++ lib/lp/app/browser/doc/base-layout.txt 2018-03-28 21:29:05 +
@@ -25,7 +25,7 @@
>>> class MainSideView(LaunchpadView):
... """A simple view to test base-layout."""
... __launchpad_facetname__ = 'overview'
-... template = ViewPageTemplateFile('testfiles/main-side.pt')
+... template = ViewPageTemplateFile('../tests/testfiles/main-side.pt')
... page_title = 'Test base-layout: main_side'
The main_side layout uses all the defined features of the base-layout
@@ -57,7 +57,7 @@
>>> class MainOnlyView(LaunchpadView):
... """A simple view to test base-layout."""
... __launchpad_facetname__ = 'overview'
-... template = ViewPageTemplateFile('testfiles/main-only.pt')
+... template = ViewPageTemplateFile('../tests/testfiles/main-only.pt')
... page_title = 'Test base-layout: main_only'
>>> view = MainOnlyView(user, request)
@@ -80,7 +80,7 @@
>>> class SearchlessView(LaunchpadView):
... """A simple view to test base-layout."""
... __launchpad_facetname__ = 'ove
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/bing-search into lp:launchpad
I believe most issues from feedback are fixed now. Still pending: turn related doctests into unittests (and deduplicate lots of doctests, btw), which I'll work on in a following branch to unlock and hopefully land/deploy this branch ASAP. Note this branch depends on https://code.launchpad.net/~maxiberta/launchpad/generalized-sitesearch-testservice/+merge/342312. -- https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 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:~maxiberta/launchpad/generalized-sitesearch-testservice into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/generalized-sitesearch-testservice into lp:launchpad.
Commit message:
Generalize googletestservice.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/generalized-sitesearch-testservice/+merge/342312
Generalize sitesearch.googletestservice, making it a thin wrapper around new
sitesearch.testservice, while keeping the patch as short as possible. Also,
assorted improvements in tests.
This will help in reducing code duplication when the Bing search engine is
introduced.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/generalized-sitesearch-testservice into lp:launchpad.
=== modified file 'lib/lp/app/stories/launchpad-root/site-search.txt'
--- lib/lp/app/stories/launchpad-root/site-search.txt 2016-01-26 15:47:37 +
+++ lib/lp/app/stories/launchpad-root/site-search.txt 2018-03-28 17:01:31 +
@@ -1,9 +1,9 @@
Site-wide Search
-Launchpad features a site-wide search that combines Google's site-
-specific search with Launchpad's prominent objects (projects, bugs,
-teams, etc.).
+Launchpad features a site-wide search that combines an external search
+engine's site-specific search with Launchpad's prominent objects (projects,
+bugs, teams, etc.).
# Our very helpful function for printing all the page results.
@@ -81,9 +81,9 @@
3: Firefox is slow and consumes too much RAM
...
-An arbitrary search returns a list of Google search results. The user
-searches for "bug" and sees a listing of matching pages. The navigation
-states that the page is showing 1 through 20 of 25 total results.
+An arbitrary search returns a list of the search engine's search results.
+The user searches for "bug" and sees a listing of matching pages. The
+navigation states that the page is showing 1 through 20 of 25 total results.
# Use our pre-defined search results for the 'bug' search.
@@ -248,9 +248,10 @@
Searches when there is no page service
--
-Google may not be available when the search is performed. This is often
-caused by temporary connectivity problems. A message is displayed that
-explains that the search can be performed again to find matching pages.
+The search provider may not be available when the search is performed.
+This is often caused by temporary connectivity problems. A message is
+displayed that explains that the search can be performed again to find
+matching pages.
>>> search_for('gnomebaker')
>>> print find_tag_by_id(anon_browser.contents, 'no-page-service')
=== added file 'lib/lp/services/sitesearch/googletestservice.py'
--- lib/lp/services/sitesearch/googletestservice.py 1970-01-01 00:00:00 +
+++ lib/lp/services/sitesearch/googletestservice.py 2018-03-28 17:01:31 +
@@ -0,0 +1,79 @@
+#!/usr/bin/python
+#
+# Copyright 2018 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""
+This script runs a simple HTTP server. The server returns XML files
+when given certain user-configurable URLs.
+"""
+
+import logging
+import os
+
+from six.moves.BaseHTTPServer import HTTPServer
+
+from lp.services.config import config
+from lp.services.osutils import ensure_directory_exists
+from lp.services.pidfile import make_pidfile
+from lp.services.sitesearch import testservice
+
+
+# Set up basic logging.
+log = logging.getLogger(__name__)
+
+# The default service name, used by the Launchpad service framework.
+service_name = 'google-webservice'
+
+
+class GoogleRequestHandler(testservice.RequestHandler):
+default_content_type = 'text/xml; charset=UTF-8'
+log = log
+mapfile = config.google_test_service.mapfile
+content_dir = config.google_test_service.canned_response_directory
+
+
+def start_as_process():
+return testservice.start_as_process('googletestservice')
+
+
+def get_service_endpoint():
+"""Return the host and port that the service is running on."""
+return testservice.hostpair(config.google.site)
+
+
+def service_is_available():
+host, port = get_service_endpoint()
+return testservice.service_is_available(host, port)
+
+
+def wait_for_service():
+host, port = get_service_endpoint()
+return testservice.wait_for_service(host, port)
+
+
+def kill_running_process():
+global service_name
+host, port = get_service_endpoint()
+return testservice.kill_running_process(service_name, host, port)
+
+
+def main():
+"""Run the HTTP server."""
+# Redirect our service output to a log file.
+global log
+ensure_directory_exists(os.path.dirname(config.google_test_service.log))
+filelog = logging.FileHandler(config.google_test_service
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/bing-search into lp:launchpad
Updated branch: - Register ISearchService implementations with name="google" and name="bing" (defaults to "google" for now; will add a feature flag next). - Add extra doctests (sorry, copied from google implementation as we are in a rush; will migrate to proper unit tests later). -- https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 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/bing-search into lp:launchpad
Also, see a number of preparatory branches: https://code.launchpad.net/~maxiberta/launchpad/googlesearchservice-improvements/+merge/342146 https://code.launchpad.net/~maxiberta/launchpad/rename-google-as-sitesearch-extra/+merge/342148 https://code.launchpad.net/~maxiberta/launchpad/deduplicate-process_exists/+merge/342150 -- https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 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/bing-search into lp:launchpad
Replied each comment inline. Most issues are now fixed. Still missing: - Add feature flag to switch search engine. - Code deduplication. - Turn doctests into proper unit tests. - Run tests on search page with both Google and Bing. Diff comments: > > === modified file 'configs/development/launchpad-lazr.conf' > --- configs/development/launchpad-lazr.conf 2018-02-02 15:29:38 + > +++ configs/development/launchpad-lazr.conf 2018-03-17 20:34:09 + > @@ -86,6 +86,15 @@ > [google_test_service] > launch: True > > +[bing] > +# Development and the testrunner should use the stub service be default. Fixed. > +site: http://launchpad.dev:8093/bingcustomsearch/v7.0/search > +subscription_key: abcdef01234567890abcdef012345678 > +custom_config_id: 1234567890 > + > +[bing_test_service] > +launch: True > + > [gpghandler] > host: keyserver.launchpad.dev > public_host: keyserver.launchpad.dev > > === modified file 'lib/lp/app/browser/root.py' > --- lib/lp/app/browser/root.py2018-03-16 14:50:01 + > +++ lib/lp/app/browser/root.py2018-03-17 20:34:09 + > @@ -510,24 +517,24 @@ > def searchPages(self, query_terms, start=0): > """Return the up to 20 pages that match the query_terms, or None. > > -:param query_terms: The unescaped terms to query Google. > +:param query_terms: The unescaped terms to query for. > :param start: The index of the page that starts the set of pages. > -:return: A GooglBatchNavigator or None. > +:return: A SearchResultsBatchNavigator or None. > """ > if query_terms in [None, '']: > return None > -google_search = getUtility(ISearchService) > +site_search = getUtility(ISearchService) > try: > -page_matches = google_search.search( > +page_matches = site_search.search( > terms=query_terms, start=start) > -except GoogleResponseError: > -# There was a connectivity or Google service issue that means > +except (BingResponseError, GoogleResponseError): Done. Replaced BingResponseError and GoogleResponseError by SiteSearchResponseError. > +# There was a connectivity or the search service issue that means > # there is no data available at this moment. > self.has_page_service = False > return None > if len(page_matches) == 0: > return None > -navigator = GoogleBatchNavigator( > +navigator = SearchResultsBatchNavigator( > page_matches, self.request, start=start) > navigator.setHeadings(*self.batch_heading) > return navigator > @@ -589,7 +596,7 @@ > return self.start + len(self.list._window) > > > -class GoogleBatchNavigator(BatchNavigator): > +class SearchResultsBatchNavigator(BatchNavigator): Fixed. > """A batch navigator with a fixed size of 20 items per batch.""" > > _batch_factory = WindowedListBatch > > === modified file 'lib/lp/app/browser/tests/test_views.py' > --- lib/lp/app/browser/tests/test_views.py2011-12-28 17:03:06 + > +++ lib/lp/app/browser/tests/test_views.py2018-03-17 20:34:09 + > @@ -11,7 +11,7 @@ > > from lp.testing.layers import ( > DatabaseFunctionalLayer, > -GoogleLaunchpadFunctionalLayer, > +BingLaunchpadFunctionalLayer, Fixed. > ) > from lp.testing.systemdocs import ( > LayeredDocFileSuite, > > === modified file 'lib/lp/services/config/schema-lazr.conf' > --- lib/lp/services/config/schema-lazr.conf 2018-03-16 14:02:16 + > +++ lib/lp/services/config/schema-lazr.conf 2018-03-17 20:34:09 + > @@ -794,6 +794,42 @@ > # Example: help.launchpad.net login.launchapd.net > url_rewrite_exceptions: help.launchpad.net > > +[bing_test_service] > +# Run a web service stub that simulates the Bing search service. > + > +# Where are our canned JSON responses stored? > +canned_response_directory: lib/lp/services/sitesearch/tests/data/ > + > +# Which file maps service URLs to the JSON that the server returns? > +mapfile: lib/lp/services/sitesearch/tests/data/bingsearchservice-mapping.txt > + > +# Where should the service log files live? > +log: logs/bing-stub.log > + > +# Do we actually want to run the service? > +launch: False > + > +[bing] > +# site is the host and path that search requests are made to. > +# eg. https://api.cognitive.microsoft.com/bingcustomsearch/v7.0/search > +# datatype: string, a url to a host > +site: https://api.cognitive.microsoft.com/bingcustomsearch/v7.0/search > + > +# subscription_key is the Cognitive Services subscription key for > +# Bing Custom Search API. > +# datatype: string > +subscription_key: > + > +# custom_config_id is the id that identifies the custom search instance. > +# datatype: string > +custom_config_id: > + > +# url_rewrite_exceptions is a list of launchpad.net domains that must > +# not be rewritten. > +# datatype: string of space
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/deduplicate-process_exists into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/deduplicate-process_exists into lp:launchpad.
Commit message:
Deduplicate code into lp.services.osutils.process_exists().
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/deduplicate-process_exists/+merge/342150
Deduplicate code into lp.services.osutils.process_exists().
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/deduplicate-process_exists into lp:launchpad.
=== modified file 'lib/lp/scripts/utilities/killservice.py'
--- lib/lp/scripts/utilities/killservice.py 2015-10-26 12:34:50 +
+++ lib/lp/scripts/utilities/killservice.py 2018-03-26 21:44:39 +
@@ -1,6 +1,6 @@
#!../bin/py
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# This module uses relative imports.
@@ -17,6 +17,7 @@
from lp.services.config import config
from lp.services.mailman.runmailman import stop_mailman
+from lp.services.osutils import process_exists
from lp.services.pidfile import (
get_pid,
pidfile_path,
@@ -101,17 +102,6 @@
pass
-def process_exists(pid):
-"""True if the given process exists."""
-try:
-os.getpgid(pid)
-except OSError as x:
-if x.errno == 3:
-return False
-logging.error("Unknown exception from getpgid - %s", str(x))
-return True
-
-
def wait_for_pids(pids, wait, log):
"""
Wait until all signalled processes are dead, or until we hit the
=== modified file 'lib/lp/services/osutils.py'
--- lib/lp/services/osutils.py 2017-01-11 18:12:28 +
+++ lib/lp/services/osutils.py 2018-03-26 21:44:39 +
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Utilities for doing the sort of thing the os module does."""
@@ -12,6 +12,7 @@
'get_pid_from_file',
'open_for_writing',
'override_environ',
+'process_exists',
'remove_if_exists',
'remove_tree',
'two_stage_kill',
@@ -194,3 +195,17 @@
if os.path.isfile(filename) and os.access(filename, os.X_OK):
return True
return False
+
+
+def process_exists(pid):
+"""Return True if the specified process already exists."""
+try:
+os.kill(pid, 0)
+except os.error as err:
+if err.errno == errno.ESRCH:
+# All is well - the process doesn't exist.
+return False
+else:
+# We got a strange OSError, which we'll pass upwards.
+raise
+return True
=== modified file 'lib/lp/services/pidfile.py'
--- lib/lp/services/pidfile.py 2014-01-10 04:30:31 +
+++ lib/lp/services/pidfile.py 2018-03-26 21:44:39 +
@@ -1,10 +1,9 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
import atexit
-import errno
import os
from signal import (
signal,
@@ -14,6 +13,7 @@
import tempfile
from lp.services.config import config
+from lp.services.osutils import process_exists
def pidfile_path(service_name, use_config=None):
@@ -104,13 +104,9 @@
# right here at the same time. But that's sufficiently unlikely, and
# stale PIDs are sufficiently annoying, that it's a reasonable
# tradeoff.
-try:
-os.kill(pid, 0)
-except OSError as e:
-if e.errno == errno.ESRCH:
-remove_pidfile(service_name, use_config)
-return False
-raise
+if not process_exists(pid):
+remove_pidfile(service_name, use_config)
+return False
# There's a PID file and we couldn't definitively say that the
# process no longer exists. Assume that we're locked.
=== modified file 'lib/lp/services/sitesearch/tests/test_googleservice.py'
--- lib/lp/services/sitesearch/tests/test_googleservice.py 2018-03-16 14:50:01 +
+++ lib/lp/services/sitesearch/tests/test_googleservice.py 2018-03-26 21:44:39 +
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""
@@ -8,10 +8,10 @@
__metaclass__ = type
-import errno
import os
import unittest
+from lp.services.osutils import process_exists
from lp.service
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/rename-google-as-sitesearch-extra into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/rename-google-as-sitesearch-extra into lp:launchpad. Commit message: A few extra renamed classes from Google* to SiteSearch*. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~maxiberta/launchpad/rename-google-as-sitesearch-extra/+merge/342148 A few extra renamed classes from Google* to SiteSearch*. - Rename GoogleBatchNavigator as SiteSearchBatchNavigator. - Rename GoogleResponseError as SiteSearchResponseError. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/rename-google-as-sitesearch-extra into lp:launchpad. === modified file 'lib/lp/app/browser/root.py' --- lib/lp/app/browser/root.py 2018-03-16 14:50:01 + +++ lib/lp/app/browser/root.py 2018-03-26 21:14:59 + @@ -42,7 +42,7 @@ from lp.services.memcache.interfaces import IMemcacheClient from lp.services.propertycache import cachedproperty from lp.services.sitesearch.interfaces import ( -GoogleResponseError, +SiteSearchResponseError, ISearchService, ) from lp.services.statistics.interfaces.statistic import ILaunchpadStatisticSet @@ -520,14 +520,14 @@ try: page_matches = google_search.search( terms=query_terms, start=start) -except GoogleResponseError: +except SiteSearchResponseError: # There was a connectivity or Google service issue that means # there is no data available at this moment. self.has_page_service = False return None if len(page_matches) == 0: return None -navigator = GoogleBatchNavigator( +navigator = SiteSearchBatchNavigator( page_matches, self.request, start=start) navigator.setHeadings(*self.batch_heading) return navigator @@ -589,7 +589,7 @@ return self.start + len(self.list._window) -class GoogleBatchNavigator(BatchNavigator): +class SiteSearchBatchNavigator(BatchNavigator): """A batch navigator with a fixed size of 20 items per batch.""" _batch_factory = WindowedListBatch @@ -614,7 +614,7 @@ :param callback: Not used. """ results = WindowedList(results, start, results.total) -super(GoogleBatchNavigator, self).__init__(results, request, +super(SiteSearchBatchNavigator, self).__init__(results, request, start=start, size=size, callback=callback, transient_parameters=transient_parameters, force_start=force_start, range_factory=range_factory) === modified file 'lib/lp/app/browser/tests/launchpad-search-pages.txt' --- lib/lp/app/browser/tests/launchpad-search-pages.txt 2018-03-16 14:02:16 + +++ lib/lp/app/browser/tests/launchpad-search-pages.txt 2018-03-26 21:14:59 + @@ -416,7 +416,7 @@ >>> search_view.has_matches True >>> search_view.pages -<...GoogleBatchNavigator ...> +<...SiteSearchBatchNavigator ...> The GoogleSearchService may not be available due to connectivity problems. The view's has_page_service attribute reports when the search was performed @@ -444,7 +444,7 @@ >>> search_view.batch_heading (u'other page matching "launchpad"', u'other pages matching "launchpad"') -The GoogleBatchNavigator behaves like most BatchNavigators, except that +The SiteSearchBatchNavigator behaves like most BatchNavigators, except that its batch size is always 20. The size restriction conforms to Google's maximum number of results that can be returned per request. @@ -625,7 +625,7 @@ -WindowedList and GoogleBatchNavigator +WindowedList and SiteSearchBatchNavigator - The LaunchpadSearchView uses two helper classes to work with @@ -635,7 +635,7 @@ or fewer PageMatches of what could be thousands of matches. Google requires client's to make repeats request to step though the batches of matches. The Windowed list is a list that contains only a subset of its -reported size. It is used to make batches in the GoogleBatchNavigator. +reported size. It is used to make batches in the SiteSearchBatchNavigator. For example, the last batch of the 'bug' search contained 5 of the 25 matching pages. The WindowList claims to be 25 items in length, but @@ -657,14 +657,14 @@ >>> results[18, 22] [None, None, <...PageMatch ...>, <...PageMatch ...>] -The GoogleBatchNavigator restricts the batch size to 20. the 'batch' +The SiteSearchBatchNavigator restricts the batch size to 20. the 'batch' parameter that comes from the URL is ignored. For example, setting the 'batch' parameter to 100 has no affect upon the Google search or on the navigator object. ->>> from lp.app.browser.root import Goog
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/googlesearchservice-improvements into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/googlesearchservice-improvements into lp:launchpad.
Commit message:
A few improvements on GoogleSearchService.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/googlesearchservice-improvements/+merge/342146
A few improvements on GoogleSearchService.
- Raise ValueError instead of AssertionError.
- Use urllib.urlencode instead of custom code.
- Import HTTPServer from six.
- Use lp.services.osutils.remove_if_exists() instead of custom safe_unlink().
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/googlesearchservice-improvements into lp:launchpad.
=== modified file 'lib/lp/services/sitesearch/__init__.py'
--- lib/lp/services/sitesearch/__init__.py 2018-03-16 14:02:16 +
+++ lib/lp/services/sitesearch/__init__.py 2018-03-26 21:22:51 +
@@ -224,12 +224,12 @@
def _checkParameter(self, name, value, is_int=False):
"""Check that a parameter value is not None or an empty string."""
if value in (None, ''):
-raise AssertionError("Missing value for parameter '%s'." % name)
+raise ValueError("Missing value for parameter '%s'." % name)
if is_int:
try:
int(value)
except ValueError:
-raise AssertionError(
+raise ValueError(
"Value for parameter '%s' is not an int." % name)
def create_search_url(self, terms, start=0):
@@ -237,16 +237,13 @@
self._checkParameter('q', terms)
self._checkParameter('start', start, is_int=True)
self._checkParameter('cx', self.client_id)
-safe_terms = urllib.quote_plus(terms.encode('utf8'))
search_params = dict(self._default_values)
-search_params['q'] = safe_terms
+search_params['q'] = terms.encode('utf8')
search_params['start'] = start
search_params['cx'] = self.client_id
-search_param_list = []
-for name in sorted(search_params):
-value = search_params[name]
-search_param_list.append('%s=%s' % (name, value))
-query_string = '&'.join(search_param_list)
+search_param_list = [
+(name, value) for name, value in sorted(search_params.items())]
+query_string = urllib.urlencode(search_param_list)
return self.site + '?' + query_string
def _getElementsByAttributeValue(self, doc, path, name, value):
=== modified file 'lib/lp/services/sitesearch/doc/google-searchservice.txt'
--- lib/lp/services/sitesearch/doc/google-searchservice.txt 2018-03-16 14:02:16 +
+++ lib/lp/services/sitesearch/doc/google-searchservice.txt 2018-03-26 21:22:51 +
@@ -208,17 +208,17 @@
>>> google_search.create_search_url('')
Traceback (most recent call last):
...
-AssertionError: Missing value for parameter 'q'.
+ValueError: Missing value for parameter 'q'.
>>> google_search.create_search_url(None)
Traceback (most recent call last):
...
-AssertionError: Missing value for parameter 'q'.
+ValueError: Missing value for parameter 'q'.
>>> google_search.create_search_url('bugs', start='true')
Traceback (most recent call last):
...
-AssertionError: Value for parameter 'start' is not an int.
+ValueError: Value for parameter 'start' is not an int.
The term parameter in this example can be defined by passing the term
argument to the method. The argument is url encoded and used as the
=== modified file 'lib/lp/services/sitesearch/googletestservice.py'
--- lib/lp/services/sitesearch/googletestservice.py 2018-03-16 14:31:03 +
+++ lib/lp/services/sitesearch/googletestservice.py 2018-03-26 21:22:51 +
@@ -9,10 +9,6 @@
"""
-from BaseHTTPServer import (
-BaseHTTPRequestHandler,
-HTTPServer,
-)
import errno
import logging
import os
@@ -21,8 +17,16 @@
import subprocess
import time
+from six.moves.BaseHTTPServer import (
+BaseHTTPRequestHandler,
+HTTPServer,
+)
+
from lp.services.config import config
-from lp.services.osutils import ensure_directory_exists
+from lp.services.osutils import (
+ensure_directory_exists,
+remove_if_exists,
+)
from lp.services.pidfile import (
get_pid,
make_pidfile,
@@ -220,7 +224,7 @@
except ValueError:
# The file contained a mangled and invalid PID number, so we should
# clean the file up.
-safe_unlink(pidfile_path(service_name))
+remove_if_exists(pidfile_path(service_name))
else:
if pid is not None:
try:
@@ -236,20 +240,11 @@
# is probably stale, so we'll remove it to prevent trash
# from lying around in t
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/bing-search into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/bing-search into lp:launchpad has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 -- 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:~maxiberta/launchpad/bing-search into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/bing-search into lp:launchpad has been updated. Commit Message changed to: Add basic Bing Custom Search site search support. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/bing-search into 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:~maxiberta/launchpad/bing-search into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/bing-search into lp:launchpad has been updated. Description changed to: Add basic Bing Custom Search support. Should be pretty unobtrusive, while adding a basic site search implementation around Bing Custom Search (shamelessly copied from the Google site search implementation). Might need some more testing; and could use some code deduplication. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/bing-search into 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:~maxiberta/launchpad/bing-search into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/bing-search into lp:launchpad has been updated. Description changed to: Add basic Bing Custom Search support. Should be pretty unobtrusive, while adding a basic implementation around Bing Custom Search. Might need some more testing; and could use some code deduplication. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/bing-search into 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:~maxiberta/launchpad/bing-search into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/bing-search into lp:launchpad.
Commit message:
Add basic Bing Custom Search support.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/bing-search/+merge/341549
Add basic Bing Custom Search support.
This should be pretty unobtrusive, while adding a basic implementation around
Bing Custom Search. Needs more testing.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/bing-search into lp:launchpad.
=== modified file 'Makefile'
--- Makefile 2018-02-01 20:56:23 +
+++ Makefile 2018-03-16 21:18:39 +
@@ -61,6 +61,7 @@
# NB: It's important PIP_BIN only mentions things genuinely produced by pip.
PIP_BIN = \
$(PY) \
+bin/bingtestservice \
bin/build-twisted-plugin-cache \
bin/combine-css \
bin/googletestservice \
@@ -284,7 +285,7 @@
bin/test -f $(TESTFLAGS) $(TESTOPTS)
run: build inplace stop
- bin/run -r librarian,google-webservice,memcached,rabbitmq,txlongpoll \
+ bin/run -r librarian,bing-webservice,memcached,rabbitmq,txlongpoll \
-i $(LPCONFIG)
run-testapp: LPCONFIG=testrunner-appserver
@@ -297,12 +298,12 @@
start-gdb: build inplace stop support_files run.gdb
nohup gdb -x run.gdb --args bin/run -i $(LPCONFIG) \
- -r librarian,google-webservice
+ -r librarian,bing-webservice
> ${LPCONFIG}-nohup.out 2>&1 &
run_all: build inplace stop
bin/run \
- -r librarian,sftp,forker,mailman,codebrowse,google-webservice,\
+ -r librarian,sftp,forker,mailman,codebrowse,bing-webservice,\
memcached,rabbitmq,txlongpoll -i $(LPCONFIG)
run_codebrowse: compile
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2018-02-02 15:29:38 +
+++ configs/development/launchpad-lazr.conf 2018-03-16 21:18:39 +
@@ -86,6 +86,15 @@
[google_test_service]
launch: True
+[bing]
+# Development and the testrunner should use the stub service be default.
+site: http://launchpad.dev:8093/bingcustomsearch/v7.0/search
+subscription_key: abcdef01234567890abcdef012345678
+custom_config_id: 1234567890
+
+[bing_test_service]
+launch: True
+
[gpghandler]
host: keyserver.launchpad.dev
public_host: keyserver.launchpad.dev
=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf 2016-10-11 15:28:25 +
+++ configs/testrunner-appserver/launchpad-lazr.conf 2018-03-16 21:18:39 +
@@ -14,6 +14,9 @@
[google_test_service]
launch: False
+[bing_test_service]
+launch: False
+
[launchpad]
openid_provider_root: http://testopenid.dev:8085/
=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2018-01-26 22:18:38 +
+++ configs/testrunner/launchpad-lazr.conf 2018-03-16 21:18:39 +
@@ -91,6 +91,9 @@
[google]
site: http://launchpad.dev:8092/cse
+[bing]
+site: http://launchpad.dev:8093/bingcustomsearch/v7.0/search
+
[gpghandler]
upload_keys: True
host: localhost
=== modified file 'lib/lp/app/browser/root.py'
--- lib/lp/app/browser/root.py 2018-03-16 14:50:01 +
+++ lib/lp/app/browser/root.py 2018-03-16 21:18:39 +
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser code for the Launchpad root page."""
@@ -45,6 +45,13 @@
GoogleResponseError,
ISearchService,
)
+from lp.services.memcache.interfaces import IMemcacheClient
+from lp.services.propertycache import cachedproperty
+from lp.services.sitesearch.interfaces import (
+BingResponseError,
+GoogleResponseError,
+ISearchService,
+)
from lp.services.statistics.interfaces.statistic import ILaunchpadStatisticSet
from lp.services.timeout import urlfetch
from lp.services.webapp import LaunchpadView
@@ -510,24 +517,24 @@
def searchPages(self, query_terms, start=0):
"""Return the up to 20 pages that match the query_terms, or None.
-:param query_terms: The unescaped terms to query Google.
+:param query_terms: The unescaped terms to query for.
:param start: The index of the page that starts the set of pages.
-:return: A GooglBatchNavigator or None.
+:return: A SearchResultsBatchNavigator or None.
"""
if query_terms in [None, '']:
return None
-google_search = getUtility(ISearchService)
+site_search = getUtility(ISearchService)
try:
-page_matches = google_search.search(
+page_matches = site_search.search(
terms=query_terms, start=start)
-except GoogleResponseError:
-# T
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/rename-googlesearch-as-sitesearch into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/rename-googlesearch-as-sitesearch into lp:launchpad. Commit message: Rename lp.services.googlesearch as lp.services.sitesearch. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~maxiberta/launchpad/rename-googlesearch-as-sitesearch/+merge/341521 Rename lp.services.googlesearch as lp.services.sitesearch. This is the first step towards migrating the site search to Bing Custom Search. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/rename-googlesearch-as-sitesearch into lp:launchpad. === modified file 'lib/lp/app/browser/root.py' --- lib/lp/app/browser/root.py 2017-11-18 13:29:18 + +++ lib/lp/app/browser/root.py 2018-03-16 14:33:02 + @@ -39,7 +39,7 @@ from lp.registry.interfaces.product import IProductSet from lp.services.config import config from lp.services.features import getFeatureFlag -from lp.services.googlesearch.interfaces import ( +from lp.services.sitesearch.interfaces import ( GoogleResponseError, ISearchService, ) === modified file 'lib/lp/app/browser/tests/launchpad-search-pages.txt' --- lib/lp/app/browser/tests/launchpad-search-pages.txt 2018-02-02 10:02:16 + +++ lib/lp/app/browser/tests/launchpad-search-pages.txt 2018-03-16 14:33:02 + @@ -642,7 +642,7 @@ the first 20 items are None. Only the last 5 items are PageMatches. >>> from lp.app.browser.root import WindowedList ->>> from lp.services.googlesearch import GoogleSearchService +>>> from lp.services.sitesearch import GoogleSearchService >>> google_search = GoogleSearchService() >>> page_matches = google_search.search(terms='bug', start=20) === modified file 'lib/lp/scripts/runlaunchpad.py' --- lib/lp/scripts/runlaunchpad.py 2017-12-19 17:16:38 + +++ lib/lp/scripts/runlaunchpad.py 2018-03-16 14:33:02 + @@ -19,7 +19,7 @@ from lp.services.config import config from lp.services.daemons import tachandler -from lp.services.googlesearch import googletestservice +from lp.services.sitesearch import googletestservice from lp.services.mailman import runmailman from lp.services.osutils import ensure_directory_exists from lp.services.pidfile import ( === modified file 'lib/lp/services/config/schema-lazr.conf' --- lib/lp/services/config/schema-lazr.conf 2018-01-31 01:19:06 + +++ lib/lp/services/config/schema-lazr.conf 2018-03-16 14:33:02 + @@ -763,15 +763,14 @@ distroseries: experimental pocketrelease: experimental - [google_test_service] # Run a web service stub that simulates the Google search service. # Where are our canned XML responses stored? -canned_response_directory: lib/lp/services/googlesearch/tests/data/ +canned_response_directory: lib/lp/services/sitesearch/tests/data/ # Which file maps service URLs to the XML that the server returns? -mapfile: lib/lp/services/googlesearch/tests/data/mapping.txt +mapfile: lib/lp/services/sitesearch/tests/data/googlesearchservice-mapping.txt # Where should the service log files live? log: logs/google-stub.log @@ -779,7 +778,6 @@ # Do we actually want to run the service? launch: False - [google] # client_id is the unique id Launchpad was issued by Google. # datatype: string === modified file 'lib/lp/services/configure.zcml' --- lib/lp/services/configure.zcml 2015-09-18 01:32:20 + +++ lib/lp/services/configure.zcml 2018-03-16 14:33:02 + @@ -11,7 +11,7 @@ - + === renamed directory 'lib/lp/services/googlesearch' => 'lib/lp/services/sitesearch' === modified file 'lib/lp/services/sitesearch/__init__.py' --- lib/lp/services/googlesearch/__init__.py 2016-04-19 14:23:22 + +++ lib/lp/services/sitesearch/__init__.py 2018-03-16 14:33:02 + @@ -24,7 +24,7 @@ from zope.interface import implementer from lp.services.config import config -from lp.services.googlesearch.interfaces import ( +from lp.services.sitesearch.interfaces import ( GoogleResponseError, GoogleWrongGSPVersion, ISearchResult, === modified file 'lib/lp/services/sitesearch/configure.zcml' --- lib/lp/services/googlesearch/configure.zcml 2011-03-07 16:32:12 + +++ lib/lp/services/sitesearch/configure.zcml 2018-03-16 14:33:02 + @@ -6,18 +6,18 @@ xmlns="http://namespaces.zope.org/zope;> - +class="lp.services.sitesearch.PageMatch"> + - +class="lp.services.sitesearch.PageMatches"> + - +class="lp.services.sitesearch.GoogleSearchService" +provides="lp.services.sitesearch.interfaces.ISearchService"> + === modified file 'lib/lp/services/sitesearch/doc/google-searchservice.txt' --- lib/lp/services/googlesearch/doc/google-searchservice.txt 2015-03-11 11:59:27 + +++ lib/lp/services/sitesearch/doc/google-searc
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/reject-bad-ssh-keys into lp:launchpad
Review: Approve Wow, a 10 year old issue! Thanks! -- https://code.launchpad.net/~cjwatson/launchpad/reject-bad-ssh-keys/+merge/339445 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/reject-bad-ssh-keys into 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/ptuj-repr into lp:launchpad
Makes sense, thanks. -- https://code.launchpad.net/~cjwatson/launchpad/ptuj-repr/+merge/338559 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/ptuj-repr into 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/ptuj-repr into lp:launchpad
Review: Approve Looks good to me. One question though: why move those @properties from `PackageTranslationsUploadJob` to its parent class `PackageTranslationsUploadJobDerived`? -- https://code.launchpad.net/~cjwatson/launchpad/ptuj-repr/+merge/338559 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/ptuj-repr into 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:~maxiberta/launchpad/lp-1729580-2 into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/lp-1729580-2 into lp:launchpad. Commit message: Use target="_blank" in extended snap build error links. Requested reviews: Colin Watson (cjwatson) For more details, see: https://code.launchpad.net/~maxiberta/launchpad/lp-1729580-2/+merge/338562 Use target="_blank" in extended snap build error links. Fixes browsers refusing to load external links due to "X-Frame-Options: sameorigin". -- Your team Launchpad code reviewers is subscribed to branch lp:launchpad. === modified file 'lib/lp/snappy/templates/snapbuild-index.pt' --- lib/lp/snappy/templates/snapbuild-index.pt 2018-02-19 17:51:30 + +++ lib/lp/snappy/templates/snapbuild-index.pt 2018-02-22 15:53:34 + @@ -185,7 +185,7 @@ (?) ___ 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/lp-1729580 into lp:launchpad
Diff comments:
> === modified file 'lib/lp/snappy/browser/tests/test_snapbuild.py'
> --- lib/lp/snappy/browser/tests/test_snapbuild.py 2017-10-20 13:35:42
> +
> +++ lib/lp/snappy/browser/tests/test_snapbuild.py 2018-02-16 21:53:15
> +
> @@ -130,6 +130,38 @@
> text=re.compile(
> r"^\s*Store upload failed:\s+Scan failed.\s*$"
>
> +def test_store_upload_status_failed_with_extended_error_message(self):
> +build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
> +job = getUtility(ISnapStoreUploadJobSource).create(build)
> +naked_job = removeSecurityProxy(job)
> +naked_job.job._status = JobStatus.FAILED
> +naked_job.error_message = "This should not be shown."
> +naked_job.error_messages = [
> +{"message": "Scan failed.", "link": "link1"},
> +{"message": "Classic not allowed.", "link": "link2"}]
> +build_view = create_initialized_view(build, "+index")
> +built_view = build_view()
> +self.assertThat(built_view, Not(soupmatchers.HTMLContains(
> +soupmatchers.Tag(
> +"store upload status", "li",
> +attrs={"id": "store-upload-status"},
> +text=re.compile('.*This should not be shown.*')
> +self.assertThat(built_view, soupmatchers.HTMLContains(
> +soupmatchers.Within(
> +soupmatchers.Tag(
> +"store upload status", "li",
> +attrs={"id": "store-upload-status"}),
> +soupmatchers.Within(
> +soupmatchers.Tag(
> +"store upload error messages", "ul",
> +attrs={"id": "store-upload-error-messages"}),
> +soupmatchers.Within(
> +soupmatchers.Tag(
> +"store upload error message", "li"),
> +soupmatchers.Tag(
> +"store upload error link", "a",
> +text="What does this mean?"))
Done.
> +
> def test_store_upload_status_release_failed(self):
> build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
> job = getUtility(ISnapStoreUploadJobSource).create(build)
>
> === modified file 'lib/lp/snappy/model/snapstoreclient.py'
> --- lib/lp/snappy/model/snapstoreclient.py2017-06-14 10:25:23 +
> +++ lib/lp/snappy/model/snapstoreclient.py2018-02-16 21:53:15 +
> @@ -316,7 +316,11 @@
> elif "errors" in response_data:
> error_message = "\n".join(
> error["message"] for error in response_data["errors"])
> -raise ScanFailedResponse(error_message)
> +error_messages = [
> +{"message": error["message"], "link": error.get("link")}
> +for error in response_data["errors"]]
Good idea, done.
> +raise ScanFailedResponse(
> +error_message, messages=error_messages)
> elif not response_data["can_release"]:
> return response_data["url"], None
> else:
>
> === modified file 'lib/lp/snappy/templates/snapbuild-index.pt'
> --- lib/lp/snappy/templates/snapbuild-index.pt2017-04-03 12:35:03
> +
> +++ lib/lp/snappy/templates/snapbuild-index.pt2018-02-16 21:53:15
> +
> @@ -177,7 +177,15 @@
>condition="context/store_upload_status/enumvalue:FAILEDTOUPLOAD">
>Store upload failed:
> -
> + +condition="not: context/store_upload_error_messages"
> +replace="context/store_upload_error_message" />
> +
> +
> +
> + What does this mean?
Nice!
> +
> +
>
>
>
--
https://code.launchpad.net/~maxiberta/launchpad/lp-1729580/+merge/337825
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:~maxiberta/launchpad/lp-1729580 into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/lp-1729580
into lp:launchpad.
Commit message:
Expose extended error messages (with external link) for snap build jobs (LP:
#1729580).
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/lp-1729580/+merge/337825
Expose extended error messages (with external link) for snap build jobs (LP:
#1729580).
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/lp-1729580 into lp:launchpad.
=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py 2017-04-27 16:22:37 +
+++ lib/lp/snappy/interfaces/snapbuild.py 2018-02-15 22:12:44 +
@@ -40,6 +40,7 @@
Choice,
Datetime,
Int,
+Text,
TextLine,
)
@@ -213,6 +214,15 @@
"this snap build to the store."),
required=False, readonly=True))
+store_upload_error_messages = exported(Text(
+title=_("Store upload error messages"),
+description=_(
+"A list of dict(message, link) where message is an error "
+"description and link, if any, is an external link to extra "
+"details, from the last attempt to upload this snap build "
+"to the store."),
+required=False, readonly=True))
+
def getFiles():
"""Retrieve the build's `ISnapFile` records.
=== modified file 'lib/lp/snappy/interfaces/snapstoreclient.py'
--- lib/lp/snappy/interfaces/snapstoreclient.py 2017-06-14 10:25:23 +
+++ lib/lp/snappy/interfaces/snapstoreclient.py 2018-02-15 22:12:44 +
@@ -29,10 +29,12 @@
class SnapStoreError(Exception):
-def __init__(self, message="", detail=None, can_retry=False):
+def __init__(
+self, message="", detail=None, messages=None, can_retry=False):
super(SnapStoreError, self).__init__(message)
self.message = message
self.detail = detail
+self.messages = messages
self.can_retry = can_retry
=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py 2018-01-23 10:59:44 +
+++ lib/lp/snappy/model/snapbuild.py 2018-02-15 22:12:44 +
@@ -456,6 +456,11 @@
job = self.last_store_upload_job
return job and job.error_message
+@property
+def store_upload_error_messages(self):
+job = self.last_store_upload_job
+return job and job.error_messages
+
def scheduleStoreUpload(self):
"""See `ISnapBuild`."""
if not self.snap.can_upload_to_store:
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2017-06-29 16:58:01 +
+++ lib/lp/snappy/model/snapbuildjob.py 2018-02-15 22:12:44 +
@@ -232,6 +232,16 @@
self.metadata["error_detail"] = detail
@property
+def error_messages(self):
+"""See `ISnapStoreUploadJob`."""
+return self.metadata.get("error_messages")
+
+@error_messages.setter
+def error_messages(self, messages):
+"""See `ISnapStoreUploadJob`."""
+self.metadata["error_messages"] = messages
+
+@property
def store_url(self):
"""See `ISnapStoreUploadJob`."""
return self.metadata.get("store_url")
@@ -326,6 +336,7 @@
self.attempt_count <= self.max_retries):
raise RetryableSnapStoreError(e.message, detail=e.detail)
self.error_message = str(e)
+self.error_messages = getattr(e, "messages", None)
self.error_detail = getattr(e, "detail", None)
if isinstance(e, UnauthorizedUploadResponse):
mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)
=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py 2017-06-14 10:25:23 +
+++ lib/lp/snappy/model/snapstoreclient.py 2018-02-15 22:12:44 +
@@ -316,7 +316,11 @@
elif "errors" in response_data:
error_message = "\n".join(
error["message"] for error in response_data["errors"])
-raise ScanFailedResponse(error_message)
+error_messages = [
+{"message": error["message"], "link": error.get("link")}
+for error in response_data["errors"]]
+raise ScanFailedResponse(
+error_message, messages=error_messages)
elif not response_data["can_release"]:
return
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-delete-with-builds into lp:launchpad
Review: Approve Looks good to me. -- https://code.launchpad.net/~cjwatson/launchpad/snap-delete-with-builds/+merge/308767 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-delete-with-builds into 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/limit-faq-editing into lp:launchpad
Review: Approve Looks good to me. Thanks -- https://code.launchpad.net/~cjwatson/launchpad/limit-faq-editing/+merge/303658 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/limit-faq-editing into 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:~maxiberta/launchpad/bug-feed-fix-private-team into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/bug-feed-fix-private-team into lp:launchpad.
Commit message:
Prevent rendering of private team names in bugs feed.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1592186 in Launchpad itself:
"feeds.launchpad.net/ubuntu/latest-bugs.atom choked"
https://bugs.launchpad.net/launchpad/+bug/1592186
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/bug-feed-fix-private-team/+merge/301337
Prevent rendering of private team names in bugs feed.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/bug-feed-fix-private-team into lp:launchpad.
=== modified file 'lib/lp/bugs/feed/templates/bug.pt'
--- lib/lp/bugs/feed/templates/bug.pt 2009-07-17 17:59:07 +
+++ lib/lp/bugs/feed/templates/bug.pt 2016-07-27 22:49:46 +
@@ -31,7 +31,10 @@
Won't Fix
Unknown
- Person
+
+Person
+
>> assert check_entries_order(entries), (
... "Published dates are not sorted.")
+=== Private teams as assignees ===
+
+Create a private team and assign an ubuntu distro bug to that team.
+
+>>> from zope.component import getUtility
+>>> from lp.bugs.interfaces.bug import IBugSet
+>>> from lp.registry.interfaces.person import PersonVisibility
+
+>>> login('foo@canonical.com')
+>>> priv_team = factory.makeTeam(visibility=PersonVisibility.PRIVATE)
+>>> bug = getUtility(IBugSet).get(1)
+>>> print bug.title
+Firefox does not support SVG
+>>> print len(bug.bugtasks)
+3
+>>> from zope.security.proxy import removeSecurityProxy
+>>> bugtask = removeSecurityProxy(bug.bugtasks[1])
+>>> print bugtask.distribution
+
+>>> bugtask.assignee = priv_team
+>>> logout()
+
+Get the ubuntu/latest-bugs feed.
+
+>>> browser.open('http://feeds.launchpad.dev/ubuntu/latest-bugs.atom')
+>>> validate_feed(browser.contents,
+... browser.headers['content-type'], browser.url)
+No Errors
+
+>>> entries = parse_entries(browser.contents)
+>>> print len(entries)
+4
+
+The bug should be included in the feed.
+
+>>> entry = entries[3]
+>>> print extract_text(entry.title)
+[1] Firefox does not support SVG
+
+Private teams should show as '-'.
+
+>>> entry_content = BSS(
+... entry.find('content').text,
+... convertEntities=BSS.HTML_ENTITIES)
+>>> soup = BSS(entry_content.text)
+>>> print [tr.findAll('td')[4].text for tr in soup.findAll('tr')[1:4]]
+[u'Mark Shuttleworth', u'-', u'-']
== Latest bugs for a source package ==
=== modified file 'lib/lp/bugs/stories/feeds/xx-bug-html.txt'
--- lib/lp/bugs/stories/feeds/xx-bug-html.txt 2011-12-29 05:29:36 +
+++ lib/lp/bugs/stories/feeds/xx-bug-html.txt 2016-07-27 22:49:46 +
@@ -92,6 +92,33 @@
>>> get_bug_numbers(entries)
[15, 15, 9, 9, 5, 5, 5, 4, 1, 1, 1]
+=== Private teams as assignees ===
+
+Create a private team and assign a mozilla bug to that team.
+
+>>> from zope.component import getUtility
+>>> from lp.bugs.interfaces.bug import IBugSet
+>>> from lp.registry.interfaces.person import PersonVisibility
+
+>>> login('foo@canonical.com')
+>>> priv_team = factory.makeTeam(visibility=PersonVisibility.PRIVATE)
+>>> bug = getUtility(IBugSet).get(1)
+>>> print bug.title
+Firefox does not support SVG
+>>> print len(bug.bugtasks)
+3
+>>> from zope.security.proxy import removeSecurityProxy
+>>> bugtask = removeSecurityProxy(bug.bugtasks[1])
+>>> bugtask.assignee = priv_team
+>>> logout()
+
+Get the mozilla/latest-bugs feed. The previous bug should be included.
+
+>>> browser.open('http://feeds.launchpad.dev/mozilla/latest-bugs.html?'
+... 'show_column=bugtargetdisplayname')
+>>> entries = parse_entries(browser.contents)
+>>> get_bug_numbers(entries)
+[15, 15, 9, 9, 5, 5, 5, 4, 1, 1, 1]
== Latest bugs for a person ==
___
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/github-bugtracker-uppercase-in-names into lp:launchpad
Review: Approve Looks good! -- https://code.launchpad.net/~cjwatson/launchpad/github-bugtracker-uppercase-in-names/+merge/301053 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/github-bugtracker-uppercase-in-names into 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:~maxiberta/launchpad/snap-pocket-help-icon-placement into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/snap-pocket-help-icon-placement into lp:launchpad.
Commit message:
Improve placement of Snap build pocket help icon.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/snap-pocket-help-icon-placement/+merge/300959
Improve placement of Snap build pocket help icon.
Snap pocket related widgets are now rendered with `LaunchpadDropdownWidget`,
which does not wrap its contents with a ``, instead of the default
`DropdownWidget`. As a result, the help icon is placed at the right of the
dropdown selector instead of a new line.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/snap-pocket-help-icon-placement into lp:launchpad.
=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py 2016-07-21 15:37:04 +
+++ lib/lp/snappy/browser/snap.py 2016-07-22 22:03:32 +
@@ -50,6 +50,7 @@
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.app.widgets.itemswidgets import (
LabeledMultiCheckBoxWidget,
+LaunchpadDropdownWidget,
LaunchpadRadioWidget,
)
from lp.buildmaster.interfaces.processor import IProcessorSet
@@ -248,6 +249,7 @@
custom_widget('archive', SnapArchiveWidget)
custom_widget('distro_arch_series', LabeledMultiCheckBoxWidget)
+custom_widget('pocket', LaunchpadDropdownWidget)
help_links = {
"pocket": u"/+help-snappy/snap-build-pocket.html",
@@ -382,6 +384,7 @@
custom_widget('store_distro_series', LaunchpadRadioWidget)
custom_widget('auto_build_archive', SnapArchiveWidget)
custom_widget('store_channels', LabeledMultiCheckBoxWidget)
+custom_widget('auto_build_pocket', LaunchpadDropdownWidget)
help_links = {
"auto_build_pocket": u"/+help-snappy/snap-build-pocket.html",
@@ -673,6 +676,7 @@
custom_widget('vcs', LaunchpadRadioWidget)
custom_widget('git_ref', GitRefWidget)
custom_widget('auto_build_archive', SnapArchiveWidget)
+custom_widget('auto_build_pocket', LaunchpadDropdownWidget)
help_links = {
"auto_build_pocket": u"/+help-snappy/snap-build-pocket.html",
___
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/snap-pocket-fix-tests into lp:launchpad
Removed ugly string concatenation in snap pocket unit test. -- https://code.launchpad.net/~maxiberta/launchpad/snap-pocket-fix-tests/+merge/300895 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/snap-pocket-fix-tests into 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:~maxiberta/launchpad/snap-pocket-fix-tests into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/snap-pocket-fix-tests into lp:launchpad. Commit message: Fix broken Snap test. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~maxiberta/launchpad/snap-pocket-fix-tests/+merge/300895 Fix broken Snap test. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/snap-pocket-fix-tests into lp:launchpad. === modified file 'lib/lp/snappy/browser/tests/test_snap.py' --- lib/lp/snappy/browser/tests/test_snap.py 2016-07-18 15:20:59 + +++ lib/lp/snappy/browser/tests/test_snap.py 2016-07-22 14:31:03 + @@ -1335,6 +1335,9 @@ Updates Proposed Backports +(?) +The package stream within the source distribution series to use """ +"""when building the snap package. or Cancel""") main_text = self.getMainText( ___ 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/snap-pocket-help into lp:launchpad
Updated with improved wording.
Diff comments:
> === modified file 'lib/lp/snappy/browser/snap.py'
> --- lib/lp/snappy/browser/snap.py 2016-07-16 07:46:23 +
> +++ lib/lp/snappy/browser/snap.py 2016-07-20 15:26:47 +
> @@ -242,11 +242,17 @@
> Choice(vocabulary='SnapDistroArchSeries'),
> title=u'Architectures', required=True)
> pocket = Choice(
> -title=u'Pocket', vocabulary=PackagePublishingPocket,
> required=True)
> +title=u'Pocket', vocabulary=PackagePublishingPocket,
> required=True,
> +description=u'The set of packages with which this snap package '
> +'should be built.')
Used "The package stream within the source distribution series to use when
building the snap package."
>
> custom_widget('archive', SnapArchiveWidget)
> custom_widget('distro_arch_series', LabeledMultiCheckBoxWidget)
>
> +help_links = {
> +"pocket": u"/+help-snappy/snap-build-pocket.html",
> +}
> +
> @property
> def cancel_url(self):
> return canonical_url(self.context)
>
> === modified file 'lib/lp/snappy/interfaces/snap.py'
> --- lib/lp/snappy/interfaces/snap.py 2016-07-15 17:11:09 +
> +++ lib/lp/snappy/interfaces/snap.py 2016-07-20 15:26:47 +
> @@ -381,8 +381,8 @@
> title=_("Pocket for automatic builds"),
> vocabulary=PackagePublishingPocket, required=False, readonly=False,
> description=_(
> -"The pocket for which automatic builds of this snap package "
> -"should be built.")))
> +"The set of packages with which automatic builds of this snap "
> +"package should be built.")))
This text is used to build the +new-snap and +edit forms, so I had to change it.
>
> is_stale = Bool(
> title=_("Snap package is stale and is due to be rebuilt."),
--
https://code.launchpad.net/~maxiberta/launchpad/snap-pocket-help/+merge/300626
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/snap-pocket-help into lp:launchpad
Granted, the default help link icon placement is somewhat awkward. It could be improved I guess? (in a different branch). -- https://code.launchpad.net/~maxiberta/launchpad/snap-pocket-help/+merge/300626 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/snap-pocket-help into 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/restore-new-snap-links into lp:launchpad
Review: Approve +1! The usage of testscenarios makes tests much cleaner. Thanks. -- https://code.launchpad.net/~cjwatson/launchpad/restore-new-snap-links/+merge/300326 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/restore-new-snap-links into 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:~maxiberta/launchpad/named-auth-tokens-fix-apidoc into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/named-auth-tokens-fix-apidoc into lp:launchpad.
Commit message:
Fix wrong apidoc format.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-fix-apidoc/+merge/300111
Fix wrong apidoc format.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/named-auth-tokens-fix-apidoc into lp:launchpad.
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2016-07-14 16:32:29 +
+++ lib/lp/soyuz/interfaces/archive.py 2016-07-14 18:23:12 +
@@ -2133,9 +2133,9 @@
a list of dictionaries or a list of full objects.
:return: A list of `ArchiveAuthToken` objects or a dictionary of
-{name: {token, archive_url} where `name` is a token name,
-`token` is the secret and `archive_url` is the externally-usable
-archive URL including basic auth.
+{name: {token, archive_url} where `name` is a token name,
+`token` is the secret and `archive_url` is the externally-usable
+archive URL including basic auth.
"""
@call_with(as_dict=True)
___
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/fix-snap-job-mailer-perms into lp:launchpad
Review: Approve Looks good to me. -- https://code.launchpad.net/~cjwatson/launchpad/fix-snap-job-mailer-perms/+merge/300103 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-snap-job-mailer-perms into 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/named-auth-tokens-bulk-api into lp:launchpad
Also, added an optional `names` parameter to `Archive.getNamedAuthTokens()` to keep uniformity with `Archive.newNamedAuthTokens()` and `Archive.revokeNamedAuthTokens()`. -- https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-bulk-api/+merge/300016 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/named-auth-tokens-htaccess into lp:launchpad
Updated branch with fixes and improvements based on code review. -- https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-htaccess/+merge/32 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/named-auth-tokens-bulk-api into lp:launchpad
Updated branch with fixes and improvements based on code review. -- https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-bulk-api/+merge/300016 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/fix-snap-store-upload-status into lp:launchpad
Review: Approve +1! -- https://code.launchpad.net/~cjwatson/launchpad/fix-snap-store-upload-status/+merge/300091 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-snap-store-upload-status into 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/fix-snap-job-refresh-perms into lp:launchpad
Review: Approve Looks good, thanks. -- https://code.launchpad.net/~cjwatson/launchpad/fix-snap-job-refresh-perms/+merge/300077 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/fix-snap-job-refresh-perms into 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/named-auth-tokens-htaccess into lp:launchpad
Diff comments: > > === modified file 'lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py' > --- lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2014-10-29 > 06:04:09 + > +++ lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2016-07-13 > 20:42:37 + > @@ -274,6 +275,22 @@ > *extra_expr) > return new_ppa_tokens > > +def getDeactivatedNamedTokens(self, since=None): > +"""Return result set of named tokens deactivated since given time.""" > +now = datetime.now(pytz.UTC) > + > +store = IStore(ArchiveAuthToken) > +extra_expr = [] > +if since: > +extra_expr = [ArchiveAuthToken.date_deactivated >= since] > +tokens = store.find( > +ArchiveAuthToken, > +ArchiveAuthToken.name != None, > +ArchiveAuthToken.date_deactivated != None, > +ArchiveAuthToken.date_deactivated <= now, The "date_deactivated != None" shows the intention of the query more explicitly imho. As to the "<= now" condition, the idea is to allow setting expiration dates in the future (eg. see `token4` in `test_getDeactivatedNamedTokens_only_those_since_last_run`). > +*extra_expr) > +return tokens > + > def getNewPrivatePPAs(self, since=None): > """Return the recently created private PPAs.""" > store = IStore(Archive) -- https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-htaccess/+merge/32 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/named-auth-tokens-bulk-api into lp:launchpad
Diff comments:
>
> === modified file 'lib/lp/soyuz/model/archive.py'
> --- lib/lp/soyuz/model/archive.py 2016-07-14 00:25:14 +
> +++ lib/lp/soyuz/model/archive.py 2016-07-14 00:25:14 +
> @@ -1985,20 +1986,57 @@
> else:
> return archive_auth_token
>
> -def getNamedAuthToken(self, name):
> +def newNamedAuthTokens(self, names, as_dict=True):
> +"""See `IArchive`."""
> +
> +if not getFeatureFlag(NAMED_AUTH_TOKEN_FEATURE_FLAG):
> +raise NamedAuthTokenFeatureDisabled()
> +
> +# Bail if the archive isn't private
> +if not self.private:
> +raise ArchiveNotPrivate("Archive must be private.")
> +
> +# Check for duplicate names.
> +token_set = getUtility(IArchiveAuthTokenSet)
> +active_tokens = token_set.getActiveNamedTokensForArchive(self)
> +active_tokens = removeSecurityProxy(active_tokens)
Didn't find a way around this. Not sure if that's ok.
> +dup_tokens = active_tokens.find(ArchiveAuthToken.name.is_in(names))
> +dup_names = [token.name for token in dup_tokens]
> +
> +values = [
> +(name, create_token(20), self) for name in names
> +if name not in dup_names]
> +tokens = create(
> +(ArchiveAuthToken.name, ArchiveAuthToken.token,
> +ArchiveAuthToken.archive), values, get_objects=True)
> +
> +# Return all requested tokens, including duplicates.
> +tokens.extend(dup_tokens)
> +if as_dict:
> +return {token.name: token.asDict() for token in tokens}
> +else:
> +return tokens
> +
> +def getNamedAuthToken(self, name, as_dict=True):
> """See `IArchive`."""
> token_set = getUtility(IArchiveAuthTokenSet)
> auth_token = token_set.getActiveNamedTokenForArchive(self, name)
> if auth_token is not None:
> -return auth_token.asDict()
> +if as_dict:
> +return auth_token.asDict()
> +else:
> +return auth_token
> else:
> raise NotFoundError(name)
>
> -def getNamedAuthTokens(self):
> +def getNamedAuthTokens(self, as_dict=True):
> """See `IArchive`."""
> token_set = getUtility(IArchiveAuthTokenSet)
> auth_tokens = token_set.getActiveNamedTokensForArchive(self)
> -return [auth_token.asDict() for auth_token in auth_tokens]
> +if as_dict:
> +return [auth_token.asDict() for auth_token in auth_tokens]
> +else:
> +return auth_tokens
>
> def revokeNamedAuthToken(self, name):
> """See `IArchive`."""
> @@ -2009,6 +2047,14 @@
> else:
> raise NotFoundError(name)
>
> +def revokeNamedAuthTokens(self, names):
> +"""See `IArchive`."""
> +token_set = getUtility(IArchiveAuthTokenSet)
> +active_tokens = token_set.getActiveNamedTokensForArchive(self)
> +active_tokens = removeSecurityProxy(active_tokens)
Didn't find a way around this. Not sure if that's ok.
> +tokens = active_tokens.find(ArchiveAuthToken.name.is_in(names))
> +tokens.set(date_deactivated=UTC_NOW)
> +
> def newSubscription(self, subscriber, registrant, date_expires=None,
> description=None):
> """See `IArchive`."""
--
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-bulk-api/+merge/300016
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/named-auth-tokens-bulk-api into 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:~maxiberta/launchpad/named-auth-tokens-bulk-api into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/named-auth-tokens-bulk-api into lp:launchpad with
lp:~maxiberta/launchpad/named-auth-tokens-htaccess as a prerequisite.
Commit message:
Add API for bulk creation and revocation of named auth tokens.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-bulk-api/+merge/300016
Add API for bulk creation and revocation of named auth tokens.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/named-auth-tokens-bulk-api into lp:launchpad.
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2016-07-14 00:25:14 +
+++ lib/lp/soyuz/interfaces/archive.py 2016-07-14 00:25:14 +
@@ -2116,6 +2116,22 @@
"""
@operation_parameters(
+names=List(
+title=_("Authorization token names"),
+value_type=TextLine(), required=True))
+@export_write_operation()
+@operation_for_version("devel")
+def newNamedAuthTokens(names):
+"""Create named authorization tokens in bulk.
+
+:param names: A list of token names.
+
+:return: A dictionary of {name: {token, archive_url} where `name` is
+a token name, `token` is the secret and `archive_url` is the
+externally-usable archive URL including basic auth.
+"""
+
+@operation_parameters(
name=TextLine(title=_("Authorization token name"), required=True))
@export_read_operation()
@operation_for_version("devel")
@@ -2146,12 +2162,24 @@
@export_write_operation()
@operation_for_version("devel")
def revokeNamedAuthToken(name):
-"""Deactivates a named authorization token.
+"""Deactivate a named authorization token.
:param name: The identifier string for a token.
:raises NotFoundError: if no matching token could be found.
"""
+@operation_parameters(
+names=List(
+title=_("Authorization token names"),
+value_type=TextLine(), required=True))
+@export_write_operation()
+@operation_for_version("devel")
+def revokeNamedAuthTokens(names):
+"""Deactivate named authorization tokens in bulk.
+
+:param names: A list of token names.
+"""
+
class IArchiveAdmin(Interface):
"""Archive interface for operations restricted by commercial."""
=== modified file 'lib/lp/soyuz/model/archive.py'
--- lib/lp/soyuz/model/archive.py 2016-07-14 00:25:14 +
+++ lib/lp/soyuz/model/archive.py 2016-07-14 00:25:14 +
@@ -93,6 +93,7 @@
from lp.registry.model.teammembership import TeamParticipation
from lp.services.config import config
from lp.services.database.bulk import (
+create,
load_referencing,
load_related,
)
@@ -1985,20 +1986,57 @@
else:
return archive_auth_token
-def getNamedAuthToken(self, name):
+def newNamedAuthTokens(self, names, as_dict=True):
+"""See `IArchive`."""
+
+if not getFeatureFlag(NAMED_AUTH_TOKEN_FEATURE_FLAG):
+raise NamedAuthTokenFeatureDisabled()
+
+# Bail if the archive isn't private
+if not self.private:
+raise ArchiveNotPrivate("Archive must be private.")
+
+# Check for duplicate names.
+token_set = getUtility(IArchiveAuthTokenSet)
+active_tokens = token_set.getActiveNamedTokensForArchive(self)
+active_tokens = removeSecurityProxy(active_tokens)
+dup_tokens = active_tokens.find(ArchiveAuthToken.name.is_in(names))
+dup_names = [token.name for token in dup_tokens]
+
+values = [
+(name, create_token(20), self) for name in names
+if name not in dup_names]
+tokens = create(
+(ArchiveAuthToken.name, ArchiveAuthToken.token,
+ArchiveAuthToken.archive), values, get_objects=True)
+
+# Return all requested tokens, including duplicates.
+tokens.extend(dup_tokens)
+if as_dict:
+return {token.name: token.asDict() for token in tokens}
+else:
+return tokens
+
+def getNamedAuthToken(self, name, as_dict=True):
"""See `IArchive`."""
token_set = getUtility(IArchiveAuthTokenSet)
auth_token = token_set.getActiveNamedTokenForArchive(self, name)
if auth_token is not None:
-return auth_token.asDict()
+if as_dict:
+return auth_token.asDict()
+else:
+return auth_token
else:
raise NotFo
[Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/named-auth-tokens-htaccess into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/named-auth-tokens-htaccess into lp:launchpad with
lp:~maxiberta/launchpad/named-auth-tokens as a prerequisite.
Commit message:
Add support for named auth tokens in ppa htpasswd creation.
Requested reviews:
Maximiliano Bertacchini (maxiberta)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens-htaccess/+merge/32
Add support for named auth tokens in ppa htpasswd creation (including the
removal of revoked named tokens).
--
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/archivepublisher/htaccess.py'
--- lib/lp/archivepublisher/htaccess.py 2013-06-20 05:50:00 +
+++ lib/lp/archivepublisher/htaccess.py 2016-07-13 20:42:37 +
@@ -58,43 +58,51 @@
file = open(filename, "a")
try:
-for entry in users:
-user, password, salt = entry
+for user, password, salt in users:
encrypted = crypt.crypt(password, salt)
file.write("%s:%s\n" % (user, encrypted))
finally:
file.close()
-def htpasswd_credentials_for_archive(archive, tokens=None):
+def htpasswd_credentials_for_archive(archive):
"""Return credentials for an archive for use with write_htpasswd.
:param archive: An `IArchive` (must be private)
-:param tokens: Optional iterable of `IArchiveAuthToken`s.
:return: Iterable of tuples with (user, password, salt) for use with
write_htpasswd.
"""
assert archive.private, "Archive %r must be private" % archive
-if tokens is None:
-tokens = IStore(ArchiveAuthToken).find(
-(ArchiveAuthToken.person_id, ArchiveAuthToken.token),
-ArchiveAuthToken.archive == archive,
-ArchiveAuthToken.date_deactivated == None)
+tokens = IStore(ArchiveAuthToken).find(
+(ArchiveAuthToken.person_id, ArchiveAuthToken.name,
+ArchiveAuthToken.token),
+ArchiveAuthToken.archive == archive,
+ArchiveAuthToken.date_deactivated == None)
# We iterate tokens more than once - materialise it.
tokens = list(tokens)
-# The first .htpasswd entry is the buildd_secret.
-yield (BUILDD_USER_NAME, archive.buildd_secret, BUILDD_USER_NAME[:2])
-
+# Preload map with person ID to person name.
person_ids = map(itemgetter(0), tokens)
names = dict(
IStore(Person).find(
(Person.id, Person.name), Person.id.is_in(set(person_ids
-# Combine the token list with the person list, sorting by person ID
-# so the file can be compared later.
-sorted_tokens = [(token[1], names[token[0]]) for token in sorted(tokens)]
-# Iterate over tokens and write the appropriate htpasswd
-# entries for them.
-for token, person_name in sorted_tokens:
-yield (person_name, token, person_name[:2])
+
+# Format the user field by combining the token list with the person list
+# (when token has person_id) or prepending a '+' (for named tokens).
+output = []
+for person_id, token_name, token in tokens:
+if token_name:
+# A named auth token.
+output.append(('+' + token_name, token, token_name[:2]))
+else:
+# A subscription auth token.
+output.append((names[person_id], token, names[person_id][:2]))
+
+# The first .htpasswd entry is the buildd_secret.
+yield (BUILDD_USER_NAME, archive.buildd_secret, BUILDD_USER_NAME[:2])
+
+# Iterate over tokens and write the appropriate htpasswd entries for them.
+# Sort by name/person ID so the file can be compared later.
+for user, password, salt in sorted(output):
+yield (user, password, salt)
=== modified file 'lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py'
--- lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2014-10-29 06:04:09 +
+++ lib/lp/archivepublisher/scripts/generate_ppa_htaccess.py 2016-07-13 20:42:37 +
@@ -89,8 +89,7 @@
fd, temp_filename = tempfile.mkstemp(dir=pub_config.temproot)
os.close(fd)
-write_htpasswd(
-temp_filename, htpasswd_credentials_for_archive(ppa))
+write_htpasswd(temp_filename, htpasswd_credentials_for_archive(ppa))
return temp_filename
@@ -176,6 +175,7 @@
store = IStore(ArchiveSubscriber)
valid_tokens = store.find(
ArchiveAuthToken,
+ArchiveAuthToken.name == None,
ArchiveAuthToken.date_deactivated == None,
ArchiveAuthToken.archive_id == ArchiveSubscriber.archive_id,
ArchiveSubscriber.status == ArchiveSubscriberStatus.CURRENT,
@@ -186,6 +186,7 @@
# all active tokens and valid tokens.
all_active_tokens = store.find(
ArchiveAuthToken,
+ArchiveAuthToken.name == None,
ArchiveAuthToken.date_deactiv
Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/reduce-mp-timeouts into lp:launchpad
Review: Approve Looks good to me! Thanks -- https://code.launchpad.net/~cjwatson/launchpad/reduce-mp-timeouts/+merge/298967 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/reduce-mp-timeouts into 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/db-named-auth-tokens into lp:launchpad/db-devel
Added unique constraint on ArchiveAuthToken(archive,name). -- https://code.launchpad.net/~maxiberta/launchpad/db-named-auth-tokens/+merge/299433 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/db-named-auth-tokens into lp:launchpad/db-devel. ___ 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/named-auth-tokens into lp:launchpad
Updated with feature flag + fixes and improvements. -- https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432 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/db-named-auth-tokens into lp:launchpad/db-devel
Fixed the new ArchiveAuthToken index based on analysis of query execution plan. Thanks! -- https://code.launchpad.net/~maxiberta/launchpad/db-named-auth-tokens/+merge/299433 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/db-named-auth-tokens into lp:launchpad/db-devel. ___ 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/named-auth-tokens into lp:launchpad
Diff comments:
> === modified file 'lib/lp/soyuz/interfaces/archive.py'
> --- lib/lp/soyuz/interfaces/archive.py2016-01-26 15:47:37 +
> +++ lib/lp/soyuz/interfaces/archive.py2016-07-07 14:30:49 +
> @@ -2074,6 +2087,57 @@
> :param dependency: is an `IArchive` object.
> """
>
> +@operation_parameters(
> +name=TextLine(title=_("Authorization token name"), required=True))
Can we instead drop the optional params `token` and `date_created`? They are
not used anywhere.
> +@export_write_operation()
> +@operation_for_version("devel")
> +def newNamedAuthToken(name, token=None, date_created=None):
> +"""Create a new named authorization token.
> +
> +:param name: An identifier string for this token.
> +:param token: Optional unicode text to use as the token. One will be
> +generated if not given.
> +:param date_created: Optional, defaults to now.
> +
> +:return: A dictionary where the value of `token` is the secret and
> +the value of `archive_url` is the externally-usable archive URL
> +including basic auth.
> +"""
> +
> +@operation_parameters(
> +name=TextLine(title=_("Authorization token name"), required=True))
> +@export_read_operation()
> +@operation_for_version("devel")
> +def getNamedAuthToken(name):
> +"""Return a named authorization token for a given archive and name.
> +
> +:param name: The identifier string for a token.
> +
> +:return: A dictionary where the value of `token` is the secret and
> +the value of `archive_url` is the externally-usable archive URL
> +including basic auth.
> +"""
> +
> +@export_read_operation()
> +@operation_for_version("devel")
> +def getNamedAuthTokens():
> +"""Return a list of named authorization tokens for a given archive.
> +
> +:return: A list of dictionaries where the value of `token` is the
> +secret and the value of `archive_url` is the externally-usable
> +archive URL including basic auth.
> +"""
> +
> +@operation_parameters(
> +name=TextLine(title=_("Authorization token name"), required=True))
> +@export_write_operation()
> +@operation_for_version("devel")
> +def revokeNamedAuthToken(name):
> +"""Deactivates a named authorization token.
> +
> +:param name: The identifier string for a token.
> +"""
> +
>
> class IArchiveAdmin(Interface):
> """Archive interface for operations restricted by commercial."""
--
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432
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/named-auth-tokens into lp:launchpad
Will fix asap. Thanks!
Diff comments:
>
> === modified file 'lib/lp/soyuz/model/archive.py'
> --- lib/lp/soyuz/model/archive.py 2016-03-14 23:42:45 +
> +++ lib/lp/soyuz/model/archive.py 2016-07-07 14:30:49 +
> @@ -1948,6 +1950,53 @@
> IStore(ArchiveAuthToken).add(archive_auth_token)
> return archive_auth_token
>
> +def newNamedAuthToken(self, name, token=None, date_created=None):
Agreed, I'm adding the feature flag asap.
> +"""See `IArchive`."""
> +
> +# Bail if the archive isn't private
> +if not self.private:
> +raise ArchiveNotPrivate("Archive must be private.")
> +
> +if self.getNamedAuthToken(name) is not None:
> +raise DuplicateTokenName(
> +"An active token with name %s for archive %s alread exists."
> %
> +(name, self.displayname))
> +
> +# Now onto the actual token creation:
> +if token is None:
> +token = create_token(20)
> +archive_auth_token = ArchiveAuthToken()
> +archive_auth_token.archive = self
> +archive_auth_token.name = name
> +archive_auth_token.token = token
> +if date_created is not None:
> +archive_auth_token.date_created = date_created
> +IStore(ArchiveAuthToken).add(archive_auth_token)
> +return archive_auth_token.as_dict()
> +
> +def getNamedAuthToken(self, name):
> +"""See `IArchive`."""
> +token_set = getUtility(IArchiveAuthTokenSet)
> +archive_auth_token = token_set.getActiveNamedTokenForArchive(self,
> name)
> +if archive_auth_token is not None:
> +return archive_auth_token.as_dict()
> +
> +def getNamedAuthTokens(self):
> +"""See `IArchive`."""
> +token_set = getUtility(IArchiveAuthTokenSet)
> +archive_auth_tokens = token_set.getActiveNamedTokensForArchive(self)
> +return [archive_auth_token.as_dict()
> +for archive_auth_token in archive_auth_tokens]
> +
> +def revokeNamedAuthToken(self, name):
> +"""See `IArchive`."""
> +token_set = getUtility(IArchiveAuthTokenSet)
> +archive_auth_token = token_set.getActiveNamedTokenForArchive(self,
> name)
> +if archive_auth_token is not None:
> +archive_auth_token.deactivate()
> +else:
> +raise NotFoundError(name)
> +
> def newSubscription(self, subscriber, registrant, date_expires=None,
> description=None):
> """See `IArchive`."""
>
> === modified file 'lib/lp/soyuz/model/archiveauthtoken.py'
> --- lib/lp/soyuz/model/archiveauthtoken.py2015-10-21 09:37:08 +
> +++ lib/lp/soyuz/model/archiveauthtoken.py2016-07-07 14:30:49 +
> @@ -88,8 +93,18 @@
> def getActiveTokenForArchiveAndPerson(self, archive, person):
> """See `IArchiveAuthTokenSet`."""
> store = Store.of(archive)
Yep, forgot to run lint, sorry.
> -return store.find(
> -ArchiveAuthToken,
> -ArchiveAuthToken.archive == archive,
> -ArchiveAuthToken.person == person,
> -ArchiveAuthToken.date_deactivated == None).one()
> +return self.getByArchive(archive).find(
> +ArchiveAuthToken.person == person).one()
> +
> +def getActiveNamedTokenForArchive(self, archive, name):
> +"""See `IArchiveAuthTokenSet`."""
> +store = Store.of(archive)
> +return self.getByArchive(archive).find(
> +ArchiveAuthToken.name == name).one()
> +
> +def getActiveNamedTokensForArchive(self, archive):
> +"""See `IArchiveAuthTokenSet`."""
> +store = Store.of(archive)
> +return self.getByArchive(archive).find(
> +ArchiveAuthToken.name != None)
> +
>
> === modified file 'lib/lp/testing/__init__.py'
> --- lib/lp/testing/__init__.py2015-11-08 01:05:24 +
> +++ lib/lp/testing/__init__.py2016-07-07 14:30:49 +
> @@ -619,7 +619,7 @@
> self.assertIsNot(
> None, pattern.search(normalise_whitespace(text)), text)
>
> -def assertIsInstance(self, instance, assert_class):
> +def assertIsInstance(self, instance, assert_class, msg=None):
I'm sorry I didn't explain. This extra param is needed to make the builtin
assertDictEqual() happy, so that the custom one can be dropped.
> """Assert that an instance is an instance of assert_class.
>
> instance and assert_class have the same semantics as the parameters
--
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432
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:~maxiberta/launchpad/named-auth-tokens into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/named-auth-tokens into lp:launchpad has been updated. Description changed to: Add new named ArchiveAuthToken API. This is required for UA customer delivery of kernel livepatches. See required DB changes in https://code.launchpad.net/~maxiberta/launchpad/db-named-auth-tokens/+merge/299433 . Still missing: * Hide behind a feature flag (to submit in a different MP, if that's ok). Also: * Use "authorize" instead of "authorise" in Archive related classes. * Some extra cleanups. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/named-auth-tokens into 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:~maxiberta/launchpad/named-auth-tokens into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/named-auth-tokens into lp:launchpad has been updated. Description changed to: Add new named ArchiveAuthToken API. This is required for UA customer delivery of kernel livepatches. See required DB changes in https://code.launchpad.net/~maxiberta/launchpad/db-named-auth-tokens/+merge/299433 . Also: * Use "authorize" instead of "authorise" in Archive related classes. * Some extra cleanups. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432 -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/named-auth-tokens into 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:~maxiberta/launchpad/db-named-auth-tokens into lp:launchpad/db-devel
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/db-named-auth-tokens into lp:launchpad/db-devel. Commit message: Add ArchiveAuthToken.name; make ArchiveAuthToken.person nullable. Requested reviews: Launchpad code reviewers (launchpad-reviewers): db For more details, see: https://code.launchpad.net/~maxiberta/launchpad/db-named-auth-tokens/+merge/299433 Add ArchiveAuthToken.name; make ArchiveAuthToken.person nullable. This is needed by the new named auth token API required for UA customer delivery of kernel livepatches. See https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/db-named-auth-tokens into lp:launchpad/db-devel. === modified file 'database/schema/comments.sql' --- database/schema/comments.sql 2016-01-26 15:47:37 + +++ database/schema/comments.sql 2016-07-07 14:31:17 + @@ -1952,6 +1952,7 @@ COMMENT ON COLUMN ArchiveAuthToken.date_created IS 'The date and time this token was created.'; COMMENT ON COLUMN ArchiveAuthToken.date_deactivated IS 'The date and time this token was deactivated.'; COMMENT ON COLUMN ArchiveAuthToken.token IS 'The token text for this authorisation.'; +COMMENT ON COLUMN ArchiveAuthToken.name IS 'The name for this named token.'; -- ArchiveDependency COMMENT ON TABLE ArchiveDependency IS 'This table maps a given archive to all other archives it should depend on.'; === added file 'database/schema/patch-2209-79-0.sql' --- database/schema/patch-2209-79-0.sql 1970-01-01 00:00:00 + +++ database/schema/patch-2209-79-0.sql 2016-07-07 14:31:17 + @@ -0,0 +1,12 @@ +-- 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 ArchiveAuthToken +ALTER COLUMN person DROP NOT NULL, +ADD COLUMN name text; + +CREATE INDEX archiveauthtoken__name__idx ON ArchiveAuthToken USING btree (name); + +INSERT INTO LaunchpadDatabaseRevision VALUES (2209, 79, 0); ___ 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:~maxiberta/launchpad/named-auth-tokens into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/named-auth-tokens into lp:launchpad.
Commit message:
Add new named ArchiveAuthToken API.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/named-auth-tokens/+merge/299432
Add new named ArchiveAuthToken API. This is required for UA customer delivery
of kernel livepatches.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/named-auth-tokens into lp:launchpad.
=== modified file 'lib/lp/soyuz/interfaces/archive.py'
--- lib/lp/soyuz/interfaces/archive.py 2016-01-26 15:47:37 +
+++ lib/lp/soyuz/interfaces/archive.py 2016-07-07 14:30:49 +
@@ -151,6 +151,12 @@
"""Raised when creating an archive subscription for a public archive."""
+@error_status(httplib.FORBIDDEN)
+class ArchiveNotOwner(Exception):
+"""Raised when the user is not the owner or a member of the owner team
+of an archive."""
+
+
@error_status(httplib.BAD_REQUEST)
class NoTokensForTeams(Exception):
"""Raised when creating a token for a team, rather than a person."""
@@ -312,6 +318,12 @@
self._fmt % {'processor': processor.name})
+@error_status(httplib.BAD_REQUEST)
+class DuplicateTokenName(Exception):
+"""Raised when creating a named token and an active token for this archive
+ with this name already exists."""
+
+
class IArchivePublic(IPrivacy, IHasOwner):
"""An Archive interface for publicly available operations."""
# Most of this stuff should really be on View, but it's needed for
@@ -526,12 +538,12 @@
"""
def newAuthToken(person, token=None, date_created=None):
-"""Create a new authorisation token.
+"""Create a new authorization token.
-:param person: An IPerson whom this token is for
+:param person: An IPerson whom this token is for.
:param token: Optional unicode text to use as the token. One will be
-generated if not given
-:param date_created: Optional, defaults to now
+generated if not given.
+:param date_created: Optional, defaults to now.
:return: A new IArchiveAuthToken
"""
@@ -1324,7 +1336,7 @@
@operation_returns_collection_of(Interface)
@export_read_operation()
def getQueueAdminsForComponent(component_name):
-"""Return `IArchivePermission` records for authorised queue admins.
+"""Return `IArchivePermission` records for authorized queue admins.
:param component_name: An `IComponent` or textual name for the
component.
@@ -1377,7 +1389,7 @@
@export_read_operation()
@operation_for_version("devel")
def getQueueAdminsForPocket(pocket, distroseries=None):
-"""Return `IArchivePermission` records for authorised queue admins.
+"""Return `IArchivePermission` records for authorized queue admins.
:param pocket: A `PackagePublishingPocket`.
:param distroseries: An optional `IDistroSeries`.
@@ -2062,6 +2074,7 @@
:return: a `IArchiveDependency` object targeted to the context
`IArchive` requiring 'dependency' `IArchive`.
"""
+
@operation_parameters(
dependency=Reference(schema=Interface, required=True),
# Really IArchive
@@ -2074,6 +2087,57 @@
:param dependency: is an `IArchive` object.
"""
+@operation_parameters(
+name=TextLine(title=_("Authorization token name"), required=True))
+@export_write_operation()
+@operation_for_version("devel")
+def newNamedAuthToken(name, token=None, date_created=None):
+"""Create a new named authorization token.
+
+:param name: An identifier string for this token.
+:param token: Optional unicode text to use as the token. One will be
+generated if not given.
+:param date_created: Optional, defaults to now.
+
+:return: A dictionary where the value of `token` is the secret and
+the value of `archive_url` is the externally-usable archive URL
+including basic auth.
+"""
+
+@operation_parameters(
+name=TextLine(title=_("Authorization token name"), required=True))
+@export_read_operation()
+@operation_for_version("devel")
+def getNamedAuthToken(name):
+"""Return a named authorization token for a given archive and name.
+
+:param name: The identifier string for a token.
+
+:return: A dictionary where the
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/new-snap-select-processors into lp:launchpad
Updated CSS to be a little less specific. Thanks! -- 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:~maxiberta/launchpad/fix-git-merge-editstatus into lp:launchpad
Maximiliano Bertacchini has proposed merging
lp:~maxiberta/launchpad/fix-git-merge-editstatus into lp:launchpad.
Commit message:
Fix +edit-status for Git merge proposals.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1538355 in Launchpad itself: "oops when changing git-based MP status
before ajax has loaded"
https://bugs.launchpad.net/launchpad/+bug/1538355
For more details, see:
https://code.launchpad.net/~maxiberta/launchpad/fix-git-merge-editstatus/+merge/298958
Fix +edit-status for Git merge proposals (fixes lp:1538355).
(Also includes a small extra cleanup of SNAP_FEATURE_FLAG that sneaked in
recent commits.)
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~maxiberta/launchpad/fix-git-merge-editstatus into lp:launchpad.
=== modified file 'lib/lp/code/browser/branchmergeproposal.py'
--- lib/lp/code/browser/branchmergeproposal.py 2016-05-14 09:54:32 +
+++ lib/lp/code/browser/branchmergeproposal.py 2016-07-01 20:50:16 +
@@ -477,6 +477,14 @@
terms.append(SimpleTerm(status, status.name, title))
return SimpleVocabulary(terms)
+@property
+def source_revid(self):
+if IBranch.providedBy(self.context.merge_source):
+source_revid = self.context.merge_source.last_scanned_id
+else:
+source_revid = self.context.merge_source.commit_sha1
+return source_revid
+
class DiffRenderingMixin:
"""A mixin class for handling diff text."""
@@ -758,16 +766,12 @@
@property
def status_config(self):
"""The config to configure the ChoiceSource JS widget."""
-if IBranch.providedBy(self.context.merge_source):
-source_revid = self.context.merge_source.last_scanned_id
-else:
-source_revid = self.context.merge_source.commit_sha1
return simplejson.dumps({
'status_widget_items': vocabulary_to_choice_edit_items(
self._createStatusVocabulary(),
css_class_prefix='mergestatus'),
'status_value': self.context.queue_status.title,
-'source_revid': source_revid,
+'source_revid': self.source_revid,
'user_can_edit_status': check_permission(
'launchpad.Edit', self.context),
})
=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py 2016-06-03 14:08:52 +
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py 2016-07-01 20:50:16 +
@@ -1758,6 +1758,18 @@
object=MatchesStructure.byEquality(
queue_status=BranchMergeProposalStatus.NEEDS_REVIEW))]))
+def test_source_revid_bzr(self):
+view = self._createView()
+self.assertEqual(
+self.proposal.merge_source.last_scanned_id, view.source_revid)
+
+def test_source_revid_git(self):
+git_proposal = self.factory.makeBranchMergeProposalForGit()
+view = BranchMergeProposalChangeStatusView(
+git_proposal, LaunchpadTestRequest())
+self.assertEqual(
+git_proposal.merge_source.commit_sha1, view.source_revid)
+
class TestCommentAttachmentRendering(TestCaseWithFactory):
"""Test diff attachments are rendered correctly."""
=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
--- lib/lp/code/model/tests/test_gitrepository.py 2016-07-01 11:53:31 +
+++ lib/lp/code/model/tests/test_gitrepository.py 2016-07-01 20:50:16 +
@@ -1947,10 +1947,6 @@
layer = ZopelessDatabaseLayer
-def setUp(self):
-super(TestGitRepositoryMarkSnapsStale, self).setUp()
-self.useFixture(FeatureFixture({SNAP_FEATURE_FLAG: u"on"}))
-
def test_same_repository(self):
# On ref changes, snap packages using this ref become stale.
[ref] = self.factory.makeGitRefs()
=== modified file 'lib/lp/code/templates/branchmergeproposal-editstatus.pt'
--- lib/lp/code/templates/branchmergeproposal-editstatus.pt 2009-08-19 08:45:32 +
+++ lib/lp/code/templates/branchmergeproposal-editstatus.pt 2016-07-01 20:50:16 +
@@ -13,7 +13,7 @@
+ tal:attributes="value view/source_revid"/>
=== modified file 'lib/lp/codehosting/scanner/tests/test_bzrsync.py'
--- lib/lp/codehosting/scanner/tests/test_bzrsync.py 2016-06-20 20:32:36 +
+++ lib/lp/codehosting/scanner/tests/test_bzrsync.py 2016-07-01 20:50:16 +
@@ -51,7 +51,6 @@
from lp.services.database.interfaces import IStore
from lp.services.features.testing import FeatureFixture
from lp.services.osutils import override_environ
-from lp.snappy.interfaces.snap import SNAP_FEATURE_FLAG
from lp.testing import TestCaseWithFactory
from lp.testing.dbuser import (
dbuser,
@@ -753,10 +752,6
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:~maxiberta/launchpad/new-snap-select-processors into lp:launchpad
Maximiliano Bertacchini has proposed merging lp:~maxiberta/launchpad/new-snap-select-processors into lp:launchpad. Commit message: Add processor selection in new Snap form. Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~maxiberta/launchpad/new-snap-select-processors/+merge/298242 Add processor selection in new Snap form. -- Your team Launchpad code reviewers is requested to review the proposed merge of lp:~maxiberta/launchpad/new-snap-select-processors into lp:launchpad. === 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 + @@ -340,7 +340,8 @@ log_oops(e, self.request) -class SnapAddView(LaunchpadFormView, SnapAuthorizeMixin): +class SnapAddView( +LaunchpadFormView, SnapAuthorizeMixin, EnableProcessorsMixin): """View for creating snap packages.""" page_title = label = 'Create a new snap package' @@ -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.") + @property def cancel_url(self): return canonical_url(self.context) @@ -432,7 +443,8 @@ data['store_distro_series'].distro_series, data['name'], 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'], processors=data['processors'], +**kwargs) if data['store_upload']: self.requestAuthorization(snap) else: @@ -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. data['processors'].append(processor) === 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-29 21:46:52 + @@ -160,6 +160,28 @@ self.snappyseries = self.factory.makeSnappySeries( usable_distro_series=[self.distroseries]) +def setUpDistroSeries(self): +"""Set up a distroseries with some available processors.""" +distroseries = self.factory.makeUbuntuDistroSeries() +processor_names = ["386", "amd64", "hppa"] +for name in processor_names: +processor = getUtility(IProcessorSet).getByName(name) +self.factory.makeDistroArchSeries( +distroseries=distroseries, architecturetag=name, +processor=processor) +with admin_logged_in(): +self.factory.makeSnappySeries(usable_distro_series=[distroseries]) +return distroseries + +def assertProcessorControls(self, processors_control, enabled, disabled): +matchers = [ +MatchesStructure.byEquality(optionValue=name, disabled=False) +for name in enabled] +matchers.extend([ +MatchesStructure.byEquality(optionValue=name, disabled=True) +for name in disabled]) +self.assertThat(processors_control.controls, MatchesSetwise(*matchers)) + def test_initial_distroseries(self): # The initial distroseries is the newest that is current or in # development. @@ -363,6 +385,47 @@ } self.assertEqual(expected_args, parse_qs(parsed_location[3])) +def test_create_new_snap_display_processors(self): +branch = self.factory.makeAnyBranch() +distroseries = self.setUpDistroSeries() +browser = self.getViewBrowser( +branch, view_name="+new-snap", user=self.person) +processors = browser.getControl(name="field.processors") +self.assertContentEqual( +["Intel 386 (386)", "AMD 64bit (amd64)", "HPPA Processor (hppa)"], +[extract_text(option) for option in processors.displayOptions]) +self.assertContentEqual([&q
Re: [Launchpad-reviewers] [Merge] lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
* Dropped the snap.allow_new feature flag and fixed lp.snappy tests accordingly. * Synced snappyseries names with production. * Updated snappydistroseries rows to not be full cross product. * Applied changes to current.sql. Thanks! -- https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 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:~maxiberta/launchpad/snappy-sampledata into lp:launchpad
The proposal to merge lp:~maxiberta/launchpad/snappy-sampledata into lp:launchpad has been updated. Commit Message changed to: Enable snappy unconditionally; add snappy sample data for development. For more details, see: https://code.launchpad.net/~maxiberta/launchpad/snappy-sampledata/+merge/298562 -- 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
