[Launchpad-reviewers] [Merge] ~cjwatson/launchpad:update-pgkill-scripts into launchpad:master

2023-08-08 Thread Colin Watson 
Colin Watson has proposed merging ~cjwatson/launchpad:update-pgkill-scripts 
into launchpad:master.

Commit message:
Update pgkillactive and pgkillidle scripts

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Similar versions of these scripts exist in lp:losa-db-scripts and 
lp:postgresql-charm, and have been a bit more actively maintained there.  
However, it's still useful to have copies of these in the Launchpad tree, since 
the simplest way to migrate some current cron jobs from production will be to 
run these as part of the `launchpad-admin` charm.

Sync our versions up more closely with those maintained elsewhere, making the 
following changes:

 * Exclude the current process from being killed.
 * Terminate the PostgreSQL backend process remotely using 
`pg_terminate_backend`, rather than relying on running on the database host 
itself.
 * Make the `pgkillidle -u` option match either the username or the application 
name.
 * Update `pgkillidle` for `pg_stat_activity` changes in PostgreSQL >= 9.2.
 * Make `pgkillidle` print a little more information on the process being 
killed.
 * Minor changes to docstrings and help text.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~cjwatson/launchpad:update-pgkill-scripts into launchpad:master.
diff --git a/utilities/pgkillactive.py b/utilities/pgkillactive.py
index c8e5e44..304a6cc 100755
--- a/utilities/pgkillactive.py
+++ b/utilities/pgkillactive.py
@@ -3,15 +3,12 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Kill transaction that have hung around for too long.
-"""
+"""Kill transactions that have hung around for too long."""
 
 __all__ = []
 
 import _pythonpath  # noqa: F401
 
-import os
-import signal
 import sys
 from optparse import OptionParser
 
@@ -34,7 +31,10 @@ def main():
 type="int",
 dest="max_seconds",
 default=60 * 60,
-help="Maximum seconds time connections are allowed to remain active.",
+help=(
+"Connections with a transaction older than SECS seconds will "
+"be killed. If 0, all matched connections will be killed."
+),
 )
 parser.add_option(
 "-q",
@@ -76,7 +76,10 @@ def main():
 """
 SELECT usename, pid, backend_start, xact_start
 FROM pg_stat_activity
-WHERE xact_start < CURRENT_TIMESTAMP - '%d seconds'::interval %s
+WHERE
+pid <> pg_backend_pid()
+AND xact_start < CURRENT_TIMESTAMP - '%d seconds'::interval
+%s
 ORDER BY pid
 """
 % (options.max_seconds, user_match_sql),
@@ -88,7 +91,7 @@ def main():
 if len(rows) == 0:
 if not options.quiet:
 print("No transactions to kill")
-return 0
+return 0
 
 for usename, pid, backend_start, transaction_start in rows:
 print(
@@ -101,7 +104,8 @@ def main():
 )
 )
 if not options.dry_run:
-os.kill(pid, signal.SIGTERM)
+cur.execute("SELECT pg_terminate_backend(%s)", (pid,))
+cur.close()
 return 0
 
 
diff --git a/utilities/pgkillidle.py b/utilities/pgkillidle.py
index 9985ae8..6b6df3b 100755
--- a/utilities/pgkillidle.py
+++ b/utilities/pgkillidle.py
@@ -3,15 +3,12 @@
 # Copyright 2009 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-"""Kill  in transaction connections that have hung around for too long.
-"""
+"""Kill idle-in-transaction connections that have hung around for too long."""
 
 __all__ = []
 
 import _pythonpath  # noqa: F401
 
-import os
-import signal
 import sys
 from optparse import OptionParser
 
@@ -64,17 +61,24 @@ def main():
 if len(args) > 0:
 parser.error("Too many arguments")
 
-ignore_sql = " AND usename <> %s" * len(options.ignore or [])
+ignore_sql = " AND %s NOT IN (usename, application_name)" * len(
+options.ignore or []
+)
 
 con = psycopg2.connect(options.connect_string)
 cur = con.cursor()
 cur.execute(
 """
-SELECT usename, pid, backend_start, query_start
+SELECT
+usename, application_name, datname, pid,
+backend_start, state_change, AGE(NOW(), state_change) AS age
 FROM pg_stat_activity
-WHERE query = ' in transaction'
-AND query_start < CURRENT_TIMESTAMP - '%d seconds'::interval %s
-ORDER BY pid
+WHERE
+pid <> pg_backend_pid()
+AND state = 'idle in transaction'
+AND state_change < CURRENT_TIMESTAMP - '%d seconds'::interval
+%s
+ORDER BY age
 """
 % (options.max_idle_seconds, ignore_sql),
 options.ignore,
@@ -85,20 +89,17 @

Re: [Launchpad-reviewers] [Merge] ~pelpsi/launchpad:team-subscription-bug into launchpad:master

2023-08-08 Thread Colin Watson 



Diff comments:

> diff --git a/lib/lp/services/webapp/tests/test_publication.py 
> b/lib/lp/services/webapp/tests/test_publication.py
> index a9c246a..cdc1570 100644
> --- a/lib/lp/services/webapp/tests/test_publication.py
> +++ b/lib/lp/services/webapp/tests/test_publication.py
> @@ -454,3 +461,26 @@ class TestPublisherStats(StatsMixin, 
> TestCaseWithFactory):
>  browser = self.getUserBrowser()
>  # This shouldn't raise ValueError
>  self.assertRaises(NotFound, browser.open, redirect_url)
> +
> +
> +class TestSubscriptions(BrowserTestCase, TestCaseWithFactory):

While this was fixed by a change somewhere central, it's a very domain-specific 
test and I don't think it belongs in this file.  Can you move it somewhere like 
`lp.soyuz.browser.tests.test_archivesubscription`?

> +layer = DatabaseFunctionalLayer
> +
> +def test_subscriptions(self):
> +owner = self.factory.makePerson()
> +canonicals = []
> +canonicals.append(self.factory.makePerson())
> +
> +self.factory.makeTeam(name="canonical", members=canonicals)
> +ppa = self.factory.makeArchive(owner=owner, private=True)
> +
> +self.assertRaises(
> +Unauthorized, self.getViewBrowser, ppa, user=canonicals[0]
> +)
> +
> +browser = self.getViewBrowser(
> +ppa, view_name="+subscriptions", user=owner
> +)
> +browser.getControl(name="field.subscriber").value = "canonical"
> +browser.getControl("Add").click()
> +browser = self.getViewBrowser(ppa, user=canonicals[0])


-- 
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/448706
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pelpsi/launchpad:team-subscription-bug 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] ~pelpsi/launchpad:team-subscription-bug into launchpad:master

2023-08-08 Thread Simone Pelosi 
Simone Pelosi has proposed merging ~pelpsi/launchpad:team-subscription-bug into 
launchpad:master.

Commit message:
Flushing db before txn.commit() and txn.abort()


Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~pelpsi/launchpad/+git/launchpad/+merge/448706
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~pelpsi/launchpad:team-subscription-bug into launchpad:master.
diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index 1557a46..fb51750 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -50,6 +50,7 @@ from lp.services.database.interfaces import (
 IStoreSelector,
 )
 from lp.services.database.policy import LaunchpadDatabasePolicy
+from lp.services.database.sqlbase import flush_database_updates
 from lp.services.features.flags import NullFeatureController
 from lp.services.oauth.interfaces import IOAuthSignedRequest
 from lp.services.statsd.interfaces.statsd_client import IStatsdClient
@@ -504,6 +505,7 @@ class LaunchpadBrowserPublication(
 
 # Abort the transaction on a read-only request.
 # NOTHING AFTER THIS SHOULD CAUSE A RETRY.
+flush_database_updates()
 if request.method in ["GET", "HEAD"]:
 self.finishReadOnlyRequest(txn)
 elif txn.isDoomed():
diff --git a/lib/lp/services/webapp/tests/test_publication.py b/lib/lp/services/webapp/tests/test_publication.py
index a9c246a..cdc1570 100644
--- a/lib/lp/services/webapp/tests/test_publication.py
+++ b/lib/lp/services/webapp/tests/test_publication.py
@@ -14,6 +14,7 @@ from zope.component import getUtility
 from zope.interface import directlyProvides
 from zope.publisher.interfaces import NotFound, Retry
 from zope.publisher.publish import publish
+from zope.security.interfaces import Unauthorized
 from zope.security.management import thread_local as zope_security_thread_local
 
 import lp.services.webapp.adapter as dbadapter
@@ -38,7 +39,13 @@ from lp.services.webapp.servers import (
 WebServicePublication,
 )
 from lp.services.webapp.vhosts import allvhosts
-from lp.testing import ANONYMOUS, TestCase, TestCaseWithFactory, login
+from lp.testing import (
+ANONYMOUS,
+BrowserTestCase,
+TestCase,
+TestCaseWithFactory,
+login,
+)
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.layers import DatabaseFunctionalLayer
 
@@ -454,3 +461,26 @@ class TestPublisherStats(StatsMixin, TestCaseWithFactory):
 browser = self.getUserBrowser()
 # This shouldn't raise ValueError
 self.assertRaises(NotFound, browser.open, redirect_url)
+
+
+class TestSubscriptions(BrowserTestCase, TestCaseWithFactory):
+layer = DatabaseFunctionalLayer
+
+def test_subscriptions(self):
+owner = self.factory.makePerson()
+canonicals = []
+canonicals.append(self.factory.makePerson())
+
+self.factory.makeTeam(name="canonical", members=canonicals)
+ppa = self.factory.makeArchive(owner=owner, private=True)
+
+self.assertRaises(
+Unauthorized, self.getViewBrowser, ppa, user=canonicals[0]
+)
+
+browser = self.getViewBrowser(
+ppa, view_name="+subscriptions", user=owner
+)
+browser.getControl(name="field.subscriber").value = "canonical"
+browser.getControl("Add").click()
+browser = self.getViewBrowser(ppa, user=canonicals[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

Re: [Launchpad-reviewers] [Merge] ~lgp171188/turnip:revamp-development-environment-setup-docs into turnip:master

2023-08-08 Thread Guruprasad 



Diff comments:

> diff --git a/docs/development.rst b/docs/development.rst
> index aeb1b72..150ad9a 100644
> --- a/docs/development.rst
> +++ b/docs/development.rst
> @@ -1,35 +1,56 @@
>  Development
>  ===
>  
> +Prerequisites
> +-
> +
> +* LXD is installed and set up. See
> +  ``_
> +  for more details.
> +
> +* A working Launchpad development environment is available. See
> +  ``_ for 
> more
> +  details.
> +
> +* A local user on the Launchpad development instance with an SSH key added.

This prerequisite depends on 
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/448673

> +  Use the ``utilities/make-lp-user`` script inside the Launchpad container
> +  to create a new user account. This script looks for SSH public keys in
> +  the home directory of the user and automatically adds them to the created
> +  account.
> +
> +
>  Setup
>  -
>  
> -Create a bionic container (optional):
> +These instructions should work with Ubuntu 18.04 (bionic) or 20.04 (focal).
> +
> +Create a LXD container.
>  
>  .. code:: bash
>  
>  lxc launch ubuntu:bionic turnip-bionic -p ${USER}
>  
> -(You may want to use a profile to bind-mount your home directory as well.)
> +Replace ``bionic`` in the above command with ``focal`` to create a
> +``focal`` container.
> +
> +It is useful to use a LXD profile to bind-mount your home directory inside
> +this container. See the `Launchpad setup guide`_ for an example of how to
> +do this.
>  
> -SSH into the new container and navigate to the turnip repo.
> +.. _Launchpad setup guide:  
> https://launchpad.readthedocs.io/en/latest/how-to/running.html#create-a-lxd-container
>  
> -Create a python virtual env:
> +Log in into the new container using SSH and navigate to the turnip repo.
> +
> +Create a Python virtual environment.
>  
>  .. code:: bash
>  
> +sudo apt update
> +sudo apt install -y python3-venv
>  python3 -m venv env
>  source env/bin/activate
>  
> -.. note::
> -If you created a container, you may need to install python virtual env:
> -
> -.. code:: bash
> -
> -sudo apt-get update
> -sudo apt-get install -y python3-venv
> -
> -Run the following:
> +Run the following commands to install turnip's dependencies and bootstrap it.
>  
>  .. code:: bash
>  


-- 
https://code.launchpad.net/~lgp171188/turnip/+git/turnip/+merge/448680
Your team Launchpad code reviewers is requested to review the proposed merge of 
~lgp171188/turnip:revamp-development-environment-setup-docs into turnip: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

Re: [Launchpad-reviewers] [Merge] ~lgp171188/launchpad:make-lp-user-improvements into launchpad:master

2023-08-08 Thread Guruprasad 
> Could you please update the commit message and say "why" you did these 
> changes?


Jürgen, I have updated the git commit messages with some explanation now.
-- 
https://code.launchpad.net/~lgp171188/launchpad/+git/launchpad/+merge/448673
Your team Launchpad code reviewers is requested to review the proposed merge of 
~lgp171188/launchpad:make-lp-user-improvements 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] ~lgp171188/turnip:revamp-development-environment-setup-docs into turnip:master

2023-08-08 Thread Guruprasad 
Guruprasad has proposed merging 
~lgp171188/turnip:revamp-development-environment-setup-docs into turnip:master.

Commit message:
Revamp the development environment setup docs

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~lgp171188/turnip/+git/turnip/+merge/448680
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
~lgp171188/turnip:revamp-development-environment-setup-docs into turnip:master.
diff --git a/docs/development.rst b/docs/development.rst
index aeb1b72..150ad9a 100644
--- a/docs/development.rst
+++ b/docs/development.rst
@@ -1,35 +1,56 @@
 Development
 ===
 
+Prerequisites
+-
+
+* LXD is installed and set up. See
+  ``_
+  for more details.
+
+* A working Launchpad development environment is available. See
+  ``_ for more
+  details.
+
+* A local user on the Launchpad development instance with an SSH key added.
+  Use the ``utilities/make-lp-user`` script inside the Launchpad container
+  to create a new user account. This script looks for SSH public keys in
+  the home directory of the user and automatically adds them to the created
+  account.
+
+
 Setup
 -
 
-Create a bionic container (optional):
+These instructions should work with Ubuntu 18.04 (bionic) or 20.04 (focal).
+
+Create a LXD container.
 
 .. code:: bash
 
 lxc launch ubuntu:bionic turnip-bionic -p ${USER}
 
-(You may want to use a profile to bind-mount your home directory as well.)
+Replace ``bionic`` in the above command with ``focal`` to create a
+``focal`` container.
+
+It is useful to use a LXD profile to bind-mount your home directory inside
+this container. See the `Launchpad setup guide`_ for an example of how to
+do this.
 
-SSH into the new container and navigate to the turnip repo.
+.. _Launchpad setup guide:  https://launchpad.readthedocs.io/en/latest/how-to/running.html#create-a-lxd-container
 
-Create a python virtual env:
+Log in into the new container using SSH and navigate to the turnip repo.
+
+Create a Python virtual environment.
 
 .. code:: bash
 
+sudo apt update
+sudo apt install -y python3-venv
 python3 -m venv env
 source env/bin/activate
 
-.. note::
-If you created a container, you may need to install python virtual env:
-
-.. code:: bash
-
-sudo apt-get update
-sudo apt-get install -y python3-venv
-
-Run the following:
+Run the following commands to install turnip's dependencies and bootstrap it.
 
 .. code:: bash
 
@@ -40,7 +61,8 @@ Run the following:
 mkdir -p /var/tmp/git.launchpad.test
 
 .. note::
-If you are running a different ubuntu version on your container (e.g. focal), you might need to run:
+If you are running a different Ubuntu version on your container
+(e.g. focal), you might need to run:
 
 .. code:: bash
 
@@ -50,49 +72,41 @@ Run the following:
 Running
 ---
 
-Pack smart-http/ssh services can be started with:
+Start the pack smart-http/ssh services with:
 
 .. code:: bash
 
 make run-pack
 
-The HTTP API can be started with:
+Start the HTTP API with:
 
 .. code:: bash
 
make run-api
 
 
-Running LP locally as a Git client to Turnip
-
+Running Launchpad locally as a Git client to turnip
+---
 
-Turnip container needs to be able to talk to xmlrpc-private.launchpad.test.
+The turnip container needs to be able to communicate with
+``xmlrpc-private.launchpad.test`` for this to work.
 
-In the turnip container the hosts file needs to point to the LP container
-(x.x.x.x is the IP address of LP):
+In the turnip container, update the hosts file to point to the Launchpad
+container, where ``x.x.x.x`` is its IP address.
 
 .. code:: bash
 
 user@turnip-bionic:~/turnip$ cat /etc/hosts
-127.0.0.1 localhost
+...
 x.x.x.x launchpad.test launchpad.test answers.launchpad.test archive.launchpad.test api.launchpad.test bazaar.launchpad.test bazaar-internal.launchpad.test blueprints.launchpad.test bugs.launchpad.test code.launchpad.test feeds.launchpad.test keyserver.launchpad.test lists.launchpad.test ppa.launchpad.test private-ppa.launchpad.test testopenid.test translations.launchpad.test xmlrpc-private.launchpad.test xmlrpc.launchpad.test
-# The following lines are desirable for IPv6 capable hosts
-::1 ip6-localhost ip6-loopback
-fe00::0 ip6-localnet
-ff00::0 ip6-mcastprefix
-ff02::1 ip6-allnodes
-ff02::2 ip6-allrouters
-ff02::3 ip6-allhosts
-
-A basic test that can be performed by dropping into the turnip container shell.
-Below exception is expected as Repository ``1`` did not exist when the RPC
-call was performed, it does show however that turnip is able to resolve
-``xmlrpc-private.launchpad.test`` and there is connectivity betwe