Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-subscription into launchpad:master

2021-03-30 Thread Thiago F. Pappacena
Pushed requested change.

Diff comments:

> diff --git a/lib/lp/security.py b/lib/lp/security.py
> index a300279..3ed3429 100644
> --- a/lib/lp/security.py
> +++ b/lib/lp/security.py
> @@ -3467,6 +3468,12 @@ class ViewOCIRecipe(AnonymousAuthorization):
>  """Anyone can view an `IOCIRecipe`."""

Right! Fixing it.

>  usedfor = IOCIRecipe
>  
> +def checkUnauthenticated(self):
> +return self.obj.visibleByUser(None)
> +
> +def checkAuthenticated(self, user):
> +return self.obj.visibleByUser(user.person)
> +
>  
>  class EditOCIRecipe(AuthorizationBase):
>  permission = 'launchpad.Edit'


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399544
Your team Launchpad code reviewers is subscribed to branch 
~pappacena/launchpad:ocirecipe-private-reconcile-pillar.

___
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] ~pappacena/launchpad:upgrade-boto into launchpad:master

2021-03-30 Thread Thiago F. Pappacena
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:upgrade-boto into 
launchpad:master.

Commit message:
Upgrading boto lib and removing workarounds for public ECR image uploads

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400419

Dependencies MP: 
https://code.launchpad.net/~pappacena/lp-source-dependencies/+git/lp-source-dependencies/+merge/400418
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pappacena/launchpad:upgrade-boto into launchpad:master.
diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
index 4d51843..923f670 100644
--- a/lib/lp/oci/model/ociregistryclient.py
+++ b/lib/lp/oci/model/ociregistryclient.py
@@ -1,4 +1,4 @@
-# Copyright 2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2020-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Client for talking to an OCI registry."""
@@ -59,8 +59,6 @@ proxy_urlfetch = partial(urlfetch, use_proxy=True)
 
 
 OCI_AWS_BEARER_TOKEN_DOMAINS_FLAG = 'oci.push.aws.bearer_token_domains'
-OCI_AWS_BOT_EXTRA_MODEL_PATH = 'oci.push.aws.boto.extra_paths'
-OCI_AWS_BOT_EXTRA_MODEL_NAME = 'oci.push.aws.boto.extra_model_name'
 
 
 def is_aws_bearer_token_domain(domain):
@@ -68,7 +66,9 @@ def is_aws_bearer_token_domain(domain):
 instead of basic auth."""
 domains = getFeatureFlag(OCI_AWS_BEARER_TOKEN_DOMAINS_FLAG)
 if not domains:
-return False
+# We know that public ECR default domain is bearer token. If the
+# flag is not set, force it.
+domains = 'public.ecr.aws'
 return any(domain.endswith(i) for i in domains.split())
 
 
@@ -680,23 +680,13 @@ class AWSAuthenticatorMixin:
 
 def _getBotoClient(self):
 params = self._getClientParameters()
-if not self.should_use_aws_extra_model:
-return boto3.client('ecr', **params)
-model_path = getFeatureFlag(OCI_AWS_BOT_EXTRA_MODEL_PATH)
-model_name = getFeatureFlag(OCI_AWS_BOT_EXTRA_MODEL_NAME)
-if not model_path or not model_name:
-log.warning(
-"%s or %s feature rules are not set. Using default model." %
-(OCI_AWS_BOT_EXTRA_MODEL_PATH, OCI_AWS_BOT_EXTRA_MODEL_NAME))
-return boto3.client('ecr', **params)
-session = boto3.Session()
-session._loader.search_paths.extend([model_path])
-return session.client(model_name, **params)
+client_type = 'ecr-public' if self.is_public_ecr else 'ecr'
+return boto3.client(client_type, **params)
 
 @property
-def should_use_aws_extra_model(self):
-"""Returns True if the given registry domain requires extra boto API
-model.
+def is_public_ecr(self):
+"""Returns True if the given registry domain is a public ECR. False
+otherwhise.
 """
 domain = urlparse(self.push_rule.registry_url).netloc
 return is_aws_bearer_token_domain(domain)
diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
index e30a470..5fe9f2f 100644
--- a/lib/lp/oci/tests/test_ociregistryclient.py
+++ b/lib/lp/oci/tests/test_ociregistryclient.py
@@ -1,4 +1,4 @@
-# Copyright 2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2020-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the OCI Registry client."""
@@ -1045,7 +1045,7 @@ class TestRegistryHTTPClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
 
 instance = RegistryHTTPClient.getInstance(push_rule)
 self.assertEqual(AWSRegistryHTTPClient, type(instance))
-self.assertFalse(instance.should_use_aws_extra_model)
+self.assertFalse(instance.is_public_ecr)
 self.assertIsInstance(instance, RegistryHTTPClient)
 
 @responses.activate
@@ -1066,7 +1066,7 @@ class TestRegistryHTTPClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
 
 instance = RegistryHTTPClient.getInstance(push_rule)
 self.assertEqual(AWSRegistryBearerTokenClient, type(instance))
-self.assertTrue(instance.should_use_aws_extra_model)
+self.assertTrue(instance.is_public_ecr)
 self.assertIsInstance(instance, RegistryHTTPClient)
 
 @responses.activate
@@ -1336,26 +1336,58 @@ class TestAWSAuthenticator(OCIConfigHelperMixin, TestCaseWithFactory):
 auth.push_rule = push_rule
 self.assertRaises(OCIRegistryAuthenticationError, auth._getRegion)
 
-def test_should_use_extra_model(self):
+def test_should_use_public_ecr_boto_model(self):
 self.setConfig({
 OCI_AWS_BEARER_TOKEN_DOMAINS_FLAG: 'bearertoken.example.com'})
+boto_client_mock = self.useFixture(
+

[Launchpad-reviewers] [Merge] ~pappacena/lp-source-dependencies:upgrade-boto into lp-source-dependencies:master

2021-03-30 Thread Thiago F. Pappacena
Thiago F. Pappacena has proposed merging 
~pappacena/lp-source-dependencies:upgrade-boto into 
lp-source-dependencies:master.

Commit message:
Upgrading boto3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pappacena/lp-source-dependencies/+git/lp-source-dependencies/+merge/400418
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pappacena/lp-source-dependencies:upgrade-boto into 
lp-source-dependencies:master.
diff --git a/dist/boto3-1.17.40.tar.gz b/dist/boto3-1.17.40.tar.gz
new file mode 100644
index 000..fa25924
Binary files /dev/null and b/dist/boto3-1.17.40.tar.gz differ
diff --git a/dist/botocore-1.20.40.tar.gz b/dist/botocore-1.20.40.tar.gz
new file mode 100644
index 000..a3dee55
Binary files /dev/null and b/dist/botocore-1.20.40.tar.gz differ
diff --git a/dist/s3transfer-0.3.6.tar.gz b/dist/s3transfer-0.3.6.tar.gz
new file mode 100644
index 000..93ce15f
Binary files /dev/null and b/dist/s3transfer-0.3.6.tar.gz differ
___
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] ~cjwatson/launchpad:getPrecachedPersons-performance into launchpad:master

2021-03-30 Thread Colin Watson
Here's the query plan with this change, which looks much healthier:

QUERY PLAN  
 

 Limit  (cost=1763277.47..1763503.38 rows=76 width=3205) (actual 
time=246.653..246.774 rows=76 loops=1)
   Buffers: shared hit=30805 read=11178 dirtied=36
   CTE relevantperson
 ->  Nested Loop  (cost=0.86..1757.02 rows=1036 width=4) (actual 
time=0.084..16.539 rows=1026 loops=1)
   Buffers: shared hit=3440 read=367
   ->  Index Only Scan using teamparticipation_team_key on 
teamparticipation  (cost=0.43..54.82 rows=1036 width=4) (actual 
time=0.060..1.990 rows=1026 loops=1)
 Index Cond: (team = 2962068)
 Filter: (person <> 2962068)
 Rows Removed by Filter: 1
 Heap Fetches: 65
 Buffers: shared hit=128 read=67
   ->  Index Only Scan using person_pkey on person person_1  
(cost=0.43..1.64 rows=1 width=4) (actual time=0.013..0.013 rows=1 loops=1026)
 Index Cond: (id = teamparticipation.person)
 Heap Fetches: 34
 Buffers: shared hit=3312 read=300
   ->  Result  (cost=1760851.64..1763931.15 rows=1036 width=3205) (actual 
time=246.325..246.748 rows=301 loops=1)
 Buffers: shared hit=30805 read=11178 dirtied=36
 ->  Sort  (cost=1760851.64..1760854.23 rows=1036 width=3204) (actual 
time=222.665..222.717 rows=301 loops=1)
   Sort Key: (person_sort_key(person.displayname, person.name))
   Sort Method: top-N heapsort  Memory: 534kB
   Buffers: shared hit=30804 read=10108 dirtied=36
   ->  Nested Loop Left Join  (cost=1227.80..1760803.81 rows=1036 
width=3204) (actual time=13.180..218.892 rows=1026 loops=1)
 Buffers: shared hit=30801 read=10108 dirtied=36
 ->  Nested Loop Left Join  (cost=1227.23..1758892.09 
rows=1036 width=3065) (actual time=0.799..164.552 rows=1026 loops=1)
   Buffers: shared hit=30022 read=9607 dirtied=34
   ->  Nested Loop Left Join  (cost=1226.66..1757239.37 
rows=1036 width=2958) (actual time=0.744..148.998 rows=1026 loops=1)
 Buffers: shared hit=29613 read=9131 dirtied=29
 ->  Nested Loop Left Join  
(cost=1226.23..1756751.90 rows=1036 width=2852) (actual time=0.726..123.061 
rows=1026 loops=1)
   Buffers: shared hit=27159 read=7492 
dirtied=29
   ->  Nested Loop Left Join  
(cost=1225.80..1756279.84 rows=1036 width=2811) (actual time=0.690..101.350 
rows=1026 loops=1)
 Buffers: shared hit=24678 
read=5868 dirtied=29
 ->  Nested Loop Left Join  
(cost=1.27..3841.73 rows=1036 width=1181) (actual time=0.165..50.325 rows=1026 
loops=1)
   Buffers: shared hit=12377 
read=2450 dirtied=29
   ->  Nested Loop Left Join  
(cost=0.85..3387.95 rows=1036 width=1120) (actual time=0.140..41.158 rows=1026 
loops=1)
 Buffers: shared 
hit=9484 read=1859 dirtied=29
 ->  Nested Loop  
(cost=0.43..2936.12 rows=1036 width=1108) (actual time=0.121..29.892 rows=1026 
loops=1)
   Buffers: shared 
hit=6792 read=1121
   ->  CTE Scan on 
relevantperson  (cost=0.00..20.72 rows=1036 width=4) (actual time=0.099..17.303 
rows=1026 loops=1)
 Buffers: 
shared hit=3440 read=367
   ->  Index Scan 
using person_pkey on person  (cost=0.43..2.81 rows=1 width=1108) (actual 
time=0.011..0.011 rows=1 loops=1026)
 Index 
Cond: (id = relevantperson.id)
 Buffers: 
shared hit=3352 read=754
 ->  Index Scan using 
karmatotalcache_person_key on karmatotalcache  (cost=0.42..0.44 rows=1 
width=12) (actual time=0.010..0.010 rows=0 loops=1026)
   Index Cond: 
(person = person.id)
   Buffers: shared 
hit=2692 read=738 dirtied=29
 

Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-subscription into launchpad:master

2021-03-30 Thread Colin Watson
Review: Approve



Diff comments:

> diff --git a/lib/lp/registry/services/sharingservice.py 
> b/lib/lp/registry/services/sharingservice.py
> index 727b91c..a0e755b 100644
> --- a/lib/lp/registry/services/sharingservice.py
> +++ b/lib/lp/registry/services/sharingservice.py
> @@ -421,9 +430,19 @@ class SharingService:
>  spec for spec in specifications
>  if spec.id in visible_private_spec_ids or not spec.private]
>  
> -return (
> -visible_bugs, visible_branches, visible_gitrepositories,
> -visible_snaps, visible_specs)
> +# Load the OCI recipes.
> +visible_ocirecipes = []
> +if ocirecipes:
> +visible_ocirecipes = list(getUtility(IOCIRecipeSet).findByIds(
> +ocirecipes_ids, visible_by_user=person))
> +
> +return {
> +"bugs": visible_bugs,
> +"branches": visible_branches,
> +"gitrepositories": visible_gitrepositories,
> +"snaps": visible_snaps,
> +"specifications": visible_specs,
> +"ocirecipes": visible_ocirecipes}

Much better!

>  
>  def getInvisibleArtifacts(self, person, bugs=None, branches=None,
>gitrepositories=None):
> diff --git a/lib/lp/security.py b/lib/lp/security.py
> index a300279..3ed3429 100644
> --- a/lib/lp/security.py
> +++ b/lib/lp/security.py
> @@ -3467,6 +3468,12 @@ class ViewOCIRecipe(AnonymousAuthorization):
>  """Anyone can view an `IOCIRecipe`."""

This docstring could probably do with an update now.

>  usedfor = IOCIRecipe
>  
> +def checkUnauthenticated(self):
> +return self.obj.visibleByUser(None)
> +
> +def checkAuthenticated(self, user):
> +return self.obj.visibleByUser(user.person)
> +
>  
>  class EditOCIRecipe(AuthorizationBase):
>  permission = 'launchpad.Edit'


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399544
Your team Launchpad code reviewers is subscribed to branch 
~pappacena/launchpad:ocirecipe-private-reconcile-pillar.

___
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] ~pappacena/launchpad:ocirecipe-private-reconcile-pillar into launchpad:master

2021-03-30 Thread Colin Watson
Review: Approve


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399469
Your team Launchpad code reviewers is subscribed to branch 
~pappacena/launchpad:ocirecipe-private-accesspolicy.

___
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] ~cjwatson/launchpad:getPrecachedPersons-performance into launchpad:master

2021-03-30 Thread Colin Watson
Note that I'm still waiting for `EXPLAIN (ANALYZE, BUFFERS)` output to confirm 
that this approach will fix the timeouts on production.
-- 
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400391
Your team Launchpad code reviewers is requested to review the proposed merge of 
~cjwatson/launchpad:getPrecachedPersons-performance into launchpad:master.

___
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] ~cjwatson/launchpad:getPrecachedPersons-performance into launchpad:master

2021-03-30 Thread Colin Watson
Colin Watson has proposed merging 
~cjwatson/launchpad:getPrecachedPersons-performance into launchpad:master.

Commit message:
Fix PersonSet._getPrecachedPersons performance for large teams

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1921727 in Launchpad itself: "Error 503 when trying to iterate over 
lp.people[team].participants"
  https://bugs.launchpad.net/launchpad/+bug/1921727

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400391

Looking up membership of a large team can tip over a performance cliff where 
PostgreSQL decides to loop over `person_sorting_idx` first and select team 
membership from that, which performs badly.  Use a CTE to force it to look at 
more selective conditions first.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~cjwatson/launchpad:getPrecachedPersons-performance into launchpad:master.
diff --git a/lib/lp/registry/model/person.py b/lib/lp/registry/model/person.py
index 6785698..4127c91 100644
--- a/lib/lp/registry/model/person.py
+++ b/lib/lp/registry/model/person.py
@@ -58,6 +58,7 @@ from storm.expr import (
 Alias,
 And,
 Coalesce,
+Column,
 Desc,
 Exists,
 In,
@@ -68,6 +69,7 @@ from storm.expr import (
 Or,
 Select,
 SQL,
+Table,
 Union,
 Upper,
 With,
@@ -3912,8 +3914,8 @@ class PersonSet:
 """Lookup all members of the team with optional precaching.
 
 :param store: Provide ability to specify the store.
-:param origin: List of storm tables and joins. This list will be
-appended to. The Person table is required.
+:param origin: List of storm tables and joins. The Person table is
+required.
 :param conditions: Storm conditions for tables in origin.
 :param need_karma: The karma attribute will be cached.
 :param need_ubuntu_coc: The is_ubuntu_coc_signer attribute will be
@@ -3928,6 +3930,17 @@ class PersonSet:
 """
 if store is None:
 store = IStore(Person)
+# The conditions we've been given are almost certainly more
+# selective that anything PostgreSQL might guess at.  Use a CTE to
+# ensure that it looks at them first.
+store = store.with_(With(
+'RelevantPerson',
+Select(Person.id, where=conditions, tables=origin)))
+relevant_person = Table('RelevantPerson')
+origin = [
+Person,
+Join(relevant_person, Column('id', relevant_person) == Person.id),
+]
 columns = [Person]
 decorators = []
 if need_karma or need_api:
@@ -3999,10 +4012,10 @@ class PersonSet:
 if len(columns) == 1:
 column = columns[0]
 # Return a simple ResultSet
-return store.using(*origin).find(column, conditions)
+return store.using(*origin).find(column)
 # Adapt the result into a cached Person.
 columns = tuple(columns)
-raw_result = store.using(*origin).find(columns, conditions)
+raw_result = store.using(*origin).find(columns)
 if need_api:
 # Collections exported on the API need to be sorted in order to
 # provide stable batching.  In some ways Person.name might make
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp


[Launchpad-reviewers] [Merge] lp:~cjwatson/lpbuildbot/auto-py3 into lp:lpbuildbot

2021-03-30 Thread Colin Watson
Colin Watson has proposed merging lp:~cjwatson/lpbuildbot/auto-py3 into 
lp:lpbuildbot.

Commit message:
Automatically run py3 builds.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/lpbuildbot/auto-py3/+merge/400389

Since we're close to upgrading production, and since we now have locking 
between different builds on the same worker, it makes sense to start 
automatically running both py2 and py3 tests on branch changes.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/lpbuildbot/auto-py3 into lp:lpbuildbot.
=== modified file 'master.cfg'
--- master.cfg	2021-03-19 10:03:47 +
+++ master.cfg	2021-03-30 13:26:24 +
@@ -55,11 +55,13 @@
 from lpbuildbot.schedulers.aggregating import AggregatingScheduler
 c['schedulers'] = []
 c['schedulers'].append(AggregatingScheduler(
-name="devel", builderNames=["lp-devel-xenial"],
+name="devel",
+builderNames=["lp-devel-xenial", "lp-devel-xenial-py3"],
 branch='master',
 treeStableTimer=3*60, treeStableCount=3))
 c['schedulers'].append(AggregatingScheduler(
-name="db-devel", builderNames=["lp-db-devel-xenial"],
+name="db-devel",
+builderNames=["lp-db-devel-xenial", "lp-db-devel-xenial-py3"],
 branch='db-devel',
 treeStableTimer=3*60, treeStableCount=3))
 

___
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] ~cjwatson/launchpad:fix-legitimate-polls into launchpad:master

2021-03-30 Thread Colin Watson
Colin Watson has proposed merging ~cjwatson/launchpad:fix-legitimate-polls into 
launchpad:master.

Commit message:
Fix test failures from restricting poll creation

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/400388
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~cjwatson/launchpad:fix-legitimate-polls into launchpad:master.
diff --git a/lib/lp/registry/browser/poll.py b/lib/lp/registry/browser/poll.py
index 066407c..8c5debe 100644
--- a/lib/lp/registry/browser/poll.py
+++ b/lib/lp/registry/browser/poll.py
@@ -411,12 +411,6 @@ class PollAddView(LaunchpadFormView):
 
 page_title = 'New poll'
 
-def __init__(self, context, request):
-if not check_permission("launchpad.AnyLegitimatePerson", context):
-raise CannotCreatePoll(
-"You do not have permission to create polls.")
-super(PollAddView, self).__init__(context, request)
-
 @property
 def cancel_url(self):
 """See `LaunchpadFormView`."""
diff --git a/lib/lp/registry/browser/team.py b/lib/lp/registry/browser/team.py
index 41af691..93f121c 100644
--- a/lib/lp/registry/browser/team.py
+++ b/lib/lp/registry/browser/team.py
@@ -1541,11 +1541,13 @@ class TeamMenuMixin(PPANavigationMenuMixIn, CommonMenuLinks):
 text = 'Show polls'
 return Link(target, text, icon='info')
 
-@enabled_with_permission('launchpad.Edit')
 def add_poll(self):
 target = '+newpoll'
 text = 'Create a poll'
-return Link(target, text, icon='add')
+enabled = (
+check_permission('launchpad.Edit', self.context) and
+check_permission('launchpad.AnyLegitimatePerson', self.context))
+return Link(target, text, icon='add', enabled=enabled)
 
 @enabled_with_permission('launchpad.Edit')
 def editemail(self):
diff --git a/lib/lp/registry/browser/tests/test_poll.py b/lib/lp/registry/browser/tests/test_poll.py
index fa1674d..7da8c34 100644
--- a/lib/lp/registry/browser/tests/test_poll.py
+++ b/lib/lp/registry/browser/tests/test_poll.py
@@ -13,6 +13,7 @@ from datetime import (
 )
 import os
 
+from fixtures import FakeLogger
 import pytz
 
 from lp.registry.interfaces.poll import (
@@ -64,11 +65,23 @@ class TestPollAddView(BrowserTestCase):
 
 def test_new_user(self):
 # A brand new user cannot create polls.
+self.useFixture(FakeLogger())
 new_person = self.factory.makePerson()
 team = self.factory.makeTeam(owner=new_person)
+now = datetime.now(pytz.UTC)
+browser = self.getViewBrowser(
+team, view_name="+newpoll", user=new_person)
+browser.getControl("The unique name of this poll").value = "colour"
+browser.getControl("The title of this poll").value = "Favourite Colour"
+browser.getControl("The date and time when this poll opens").value = (
+str(now + timedelta(days=1)))
+browser.getControl("The date and time when this poll closes").value = (
+str(now + timedelta(days=2)))
+browser.getControl(
+"The proposition that is going to be voted").value = (
+"What is your favourite colour?")
 self.assertRaises(
-CannotCreatePoll,
-self.getViewBrowser, team, view_name="+newpoll", user=new_person)
+CannotCreatePoll, browser.getControl("Continue").click)
 
 def test_legitimate_user(self):
 # A user with some kind of track record can create polls.
diff --git a/lib/lp/registry/browser/tests/test_team.py b/lib/lp/registry/browser/tests/test_team.py
index 639d053..a26c701 100644
--- a/lib/lp/registry/browser/tests/test_team.py
+++ b/lib/lp/registry/browser/tests/test_team.py
@@ -667,6 +667,27 @@ class TestTeamMenu(TestCaseWithFactory):
 link = menu.configure_mailing_list()
 self.assertEqual('Configure mailing list', link.text)
 
+def test_TeamOverviewMenu_new_user_without_add_poll(self):
+# A brand new user does not see the add_poll link.
+self.pushConfig(
+'launchpad', min_legitimate_karma=5, min_legitimate_account_age=5)
+login_person(self.team.teamowner)
+menu = TeamOverviewMenu(self.team)
+self.assertNotIn(
+'add_poll',
+[link.name for link in menu.iterlinks() if link.enabled])
+
+def test_TeamOverviewMenu_legitimate_user_with_add_poll(self):
+# A user with some kind of track record sees the add_poll link.
+self.pushConfig(
+'launchpad', min_legitimate_karma=5, min_legitimate_account_age=5)
+team = self.factory.makeTeam(owner=self.factory.makePerson(karma=10))
+login_person(team.teamowner)
+menu = TeamOverviewMenu(team)
+self.assertIn(
+'add_poll',
+[link.name for link in menu.iterlinks() if link.enabled])
+
 

[Launchpad-reviewers] [Merge] ~twom/launchpad:oci-fix-tagging-tag-names into launchpad:master

2021-03-30 Thread Tom Wardill
Tom Wardill has proposed merging ~twom/launchpad:oci-fix-tagging-tag-names into 
launchpad:master.

Commit message:
Account for tag names in the correct branch format

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/400377

If you manage to select a tag from the branch picker, then we shouldn't attempt 
to upload the manifest to an invalid location.

LP:1921865
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~twom/launchpad:oci-fix-tagging-tag-names into launchpad:master.
diff --git a/lib/lp/oci/model/ociregistryclient.py b/lib/lp/oci/model/ociregistryclient.py
index 34a8c1e..8c27040 100644
--- a/lib/lp/oci/model/ociregistryclient.py
+++ b/lib/lp/oci/model/ociregistryclient.py
@@ -257,7 +257,11 @@ class OCIRegistryClient:
 """
 tags = []
 if recipe.is_valid_branch_format:
-tags.append("{}_{}".format(recipe.git_ref.name, "edge"))
+ref_name = recipe.git_ref.name
+# lp:1921865, account for tags in the correct format
+if ref_name.startswith("refs/tags/"):
+ref_name = ref_name[len("refs/tags/"):]
+tags.append("{}_{}".format(ref_name, "edge"))
 else:
 tags.append("edge")
 return tags
diff --git a/lib/lp/oci/tests/test_ociregistryclient.py b/lib/lp/oci/tests/test_ociregistryclient.py
index 10f3af8..e30a470 100644
--- a/lib/lp/oci/tests/test_ociregistryclient.py
+++ b/lib/lp/oci/tests/test_ociregistryclient.py
@@ -418,6 +418,13 @@ class TestOCIRegistryClient(OCIConfigHelperMixin, SpyProxyCallsMixin,
 self.assertThat(result, MatchesListwise(
 [Equals("v1.0-20.04_edge")]))
 
+def test_calculateTags_valid_tag(self):
+[git_ref] = self.factory.makeGitRefs(paths=["refs/tags/v1.0-20.04"])
+self.build.recipe.git_ref = git_ref
+result = self.client._calculateTags(self.build.recipe)
+self.assertThat(result, MatchesListwise(
+[Equals("v1.0-20.04_edge")]))
+
 def test_build_registry_manifest(self):
 self._makeFiles()
 preloaded_data = self.client._preloadFiles(
___
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