Merge pull request #721 from fschulze/567-proxy-to-master

Fix #567: the ``pip search`` view is no longer proxied to the master.

.travis.yml

@@ -40,7 +40,8 @@ jobs:
         - devpi index -c "${DEVPI_INDEX}" bases=root/pypi
         - devpi use "${DEVPI_INDEX}"
         - devpi push --index root/pypi devpi-server==4.0.0 "${DEVPI_INDEX}"
-        - devpi push --index root/pypi devpi-server==4.2.1 "${DEVPI_INDEX}"
+# BBB,XXX uncomment when devpi-server 5.2.0 is released
+#        - devpi push --index root/pypi devpi-server==5.2.0 "${DEVPI_INDEX}"
         - cd ${TRAVIS_BUILD_DIR}/common
         - yes | towncrier
         - devpi upload

server/devpi_server/__init__.py

@@ -1 +1 @@
-__version__ = '5.1.1.dev0'
+__version__ = '5.2.0.dev2'

server/devpi_server/main.py

@@ -386,6 +386,26 @@ def httpget(self, url, allow_redirects, timeout=None, extra_headers=None):
             threadlog.warn("HTTPError during httpget of %s at %s", url, location)
             return FatalResponse(url, repr(sys.exc_info()[1]))
 
+    def view_deriver(self, view, info):
+        if self.is_replica():
+            if info.options.get('is_mutating', True):
+                from .model import ensure_list
+                from .replica import proxy_view_to_master
+                from .views import is_mutating_http_method
+                request_methods = info.options['request_method']
+                if request_methods is None:
+                    request_methods = []
+                for request_method in ensure_list(request_methods):
+                    if is_mutating_http_method(request_method):
+                        # we got a view which uses a mutating method and isn't
+                        # marked to be excluded, so we replace the view with
+                        # one that proxies to the master, because a replica
+                        # must not modify its database except via the
+                        # replication protocol
+                        return proxy_view_to_master
+        return view
+    view_deriver.options = ('is_mutating',)
+
     def create_app(self):
         from devpi_server.view_auth import DevpiAuthenticationPolicy
         from devpi_server.views import ContentTypePredicate
@@ -394,6 +414,7 @@ def create_app(self):
         from pkg_resources import get_distribution
         from pyramid.authorization import ACLAuthorizationPolicy
         from pyramid.config import Configurator
+        from pyramid.viewderivers import INGRESS
         log = self.log
         log.debug("creating application in process %s", os.getpid())
         pyramid_config = Configurator(root_factory='devpi_server.view_auth.RootFactory')
@@ -430,6 +451,7 @@ def create_app(self):
                 config=self.config,
                 pyramid_config=pyramid_config)
 
+        pyramid_config.add_view_deriver(self.view_deriver, under=INGRESS)
         pyramid_config.add_view_predicate('content_type', ContentTypePredicate)
 
         pyramid_config.add_route("/+changelog/{serial}",
@@ -466,12 +488,6 @@ def create_app(self):
 
         # register tweens for logging, transaction and replication
         pyramid_config.add_tween("devpi_server.views.tween_request_logging")
-        if self.is_replica():
-            pyramid_config.add_tween(
-                "devpi_server.replica.tween_replica_proxy",
-                over="devpi_server.views.tween_keyfs_transaction",
-                under="devpi_server.views.tween_request_logging",
-            )
         pyramid_config.add_tween("devpi_server.views.tween_keyfs_transaction",
             under="devpi_server.views.tween_request_logging"
         )

server/devpi_server/replica.py

@@ -13,7 +13,7 @@
 from . import mythread
 from .fileutil import BytesForHardlink, dumps, loads, rename
 from .log import thread_push_log, threadlog
-from .views import is_mutating_http_method, H_MASTER_UUID, make_uuid_headers
+from .views import H_MASTER_UUID, make_uuid_headers
 from .model import UpstreamError
 
 H_REPLICA_UUID = str("X-DEVPI-REPLICA-UUID")
@@ -341,17 +341,6 @@ def __call__(self, ev):
                     cache_projectnames.add(project)
 
 
-def tween_replica_proxy(handler, registry):
-    xom = registry["xom"]
-    def handle_replica_proxy(request):
-        assert not hasattr(xom.keyfs, "tx"), "no tx should be ongoing"
-        if is_mutating_http_method(request.method):
-            return proxy_write_to_master(xom, request)
-        else:
-            return handler(request)
-    return handle_replica_proxy
-
-
 hop_by_hop = frozenset((
     'connection',
     'keep-alive',
@@ -429,6 +418,13 @@ def proxy_write_to_master(xom, request):
                     headers=headers)
 
 
+def proxy_view_to_master(context, request):
+    xom = request.registry["xom"]
+    tx = getattr(xom.keyfs, "tx", None)
+    assert getattr(tx, "write", False) is False, "there should be no write transaction"
+    return proxy_write_to_master(xom, request)
+
+
 class ReplicationErrors:
     def __init__(self, serverdir):
         self.errorsfn = serverdir.join(".replicationerrors")

server/setup.py

@@ -47,7 +47,7 @@ def get_changelog():
       keywords="pypi realtime cache server",
       long_description="\n\n".join([README, CHANGELOG]),
       url="http://doc.devpi.net",
-      version='5.1.1.dev0',
+      version='5.2.0.dev2',
       maintainer="Holger Krekel, Florian Schulze",
       maintainer_email="holger@merlinux.eu",
       packages=find_packages(),

server/test_devpi_server/test_replica.py

@@ -12,10 +12,9 @@
 from devpi_server.replica import ImportFileReplica
 from devpi_server.replica import MasterChangelogRequest
 from devpi_server.replica import ReplicaThread
-from devpi_server.replica import tween_replica_proxy
+from devpi_server.replica import proxy_view_to_master
 from devpi_server.views import iter_remote_file_replica
 from pyramid.httpexceptions import HTTPNotFound
-from pyramid.response import Response
 
 
 pytestmark = [pytest.mark.notransaction]
@@ -317,25 +316,17 @@ def test_clean_response_headers(mock):
     assert 'EGG' in headers
 
 
-class TestTweenReplica:
-    def test_nowrite(self, xom, blank_request):
-        l = []
-        def wrapped_handler(request):
-            l.append(xom.keyfs.get_current_serial())
-            return Response("")
-        handler = tween_replica_proxy(wrapped_handler, {"xom": xom})
-        handler(blank_request())
-        assert l == [xom.keyfs.get_current_serial()]
-
+class TestProxyViewToMaster:
     def test_write_proxies(self, makexom, blank_request, reqmock, monkeypatch):
         xom = makexom(["--master", "http://localhost"])
         reqmock.mock("http://localhost/blankpath",
                      code=200, headers={"X-DEVPI-SERIAL": "10"})
         l = []
         monkeypatch.setattr(xom.keyfs, "wait_tx_serial",
                             lambda x: l.append(x))
-        handler = tween_replica_proxy(None, {"xom": xom})
-        response = handler(blank_request(method="PUT"))
+        request = blank_request(method="PUT")
+        request.registry = dict(xom=xom)
+        response = proxy_view_to_master(None, request)
         assert response.headers.get("X-DEVPI-SERIAL") == "10"
         assert l == [10]
 
@@ -346,8 +337,9 @@ def test_preserve_reason(self, makexom, blank_request, reqmock, monkeypatch):
         l = []
         monkeypatch.setattr(xom.keyfs, "wait_tx_serial",
                             lambda x: l.append(x))
-        handler = tween_replica_proxy(None, {"xom": xom})
-        response = handler(blank_request(method="PUT"))
+        request = blank_request(method="PUT")
+        request.registry = dict(xom=xom)
+        response = proxy_view_to_master(None, request)
         assert response.status == "200 GOOD"
 
     def test_write_proxies_redirect(self, makexom, blank_request, reqmock, monkeypatch):
@@ -360,11 +352,11 @@ def test_write_proxies_redirect(self, makexom, blank_request, reqmock, monkeypat
         l = []
         monkeypatch.setattr(xom.keyfs, "wait_tx_serial",
                             lambda x: l.append(x))
-        handler = tween_replica_proxy(None, {"xom": xom})
         # normally the app is wrapped by OutsideURLMiddleware, since this is
         # not the case here, we have to set the host explicitly
-        response = handler(
-            blank_request(method="PUT", headers=dict(host='my.domain')))
+        request = blank_request(method="PUT", headers=dict(host='my.domain'))
+        request.registry = dict(xom=xom)
+        response = proxy_view_to_master(None, request)
         assert response.headers.get("X-DEVPI-SERIAL") == "10"
         assert response.headers.get("location") == "http://my.domain/hello"
         assert l == [10]
@@ -379,8 +371,9 @@ def test_hop_headers(self, makexom, blank_request, reqmock, monkeypatch):
                         "X-DEVPI-SERIAL": "0"})
         monkeypatch.setattr(xom.keyfs, "wait_tx_serial",
                             lambda x: x)
-        handler = tween_replica_proxy(None, {"xom": xom})
-        response = handler(blank_request(method="PUT"))
+        request = blank_request(method="PUT")
+        request.registry = dict(xom=xom)
+        response = proxy_view_to_master(None, request)
         assert 'connection' not in response.headers
         assert 'foo' not in response.headers
         assert 'keep-alive' not in response.headers

web/devpi_web/__init__.py

@@ -1 +1 @@
-__version__ = '3.5.2'
+__version__ = '4.0.0.dev3'

web/devpi_web/views.py

@@ -1018,7 +1018,7 @@ def query_from_xmlrpc(self, body):
 
     @view_config(
         route_name="/{user}/{index}/", request_method="POST",
-        content_type="text/xml")
+        content_type="text/xml", is_mutating=False)
     def xmlrpc_search(self):
         try:
             query = self.query_from_xmlrpc(self.request.body)

web/news/567.bugfix

@@ -0,0 +1 @@
+Fix #567: the ``pip search`` view is no longer proxied to the master.

web/news/server.other

@@ -0,0 +1 @@
+The minimum supported version of devpi-server is now 5.2.0.

web/setup.py

@@ -28,7 +28,7 @@ def get_changelog():
     description="devpi-web: a web view for devpi-server",
     long_description="\n\n".join([README, CHANGELOG]),
     url="http://doc.devpi.net",
-    version='3.5.2',
+    version='4.0.0.dev3',
     maintainer="Holger Krekel, Florian Schulze",
     maintainer_email="holger@merlinux.eu",
     license="MIT",
@@ -54,7 +54,7 @@ def get_changelog():
         'Whoosh<3',
         'beautifulsoup4>=4.3.2',
         'defusedxml',
-        'devpi-server>=4.2.1',
+        'devpi-server>=5.2dev',
         'devpi-common>=3.2.0',
         'docutils>=0.11',
         'pygments>=1.6',

web/tox.ini

@@ -1,5 +1,5 @@
 [tox]
-envlist = py27{,-keyfs,-server421},py34,py35,pypy
+envlist = py27{,-keyfs,-server520},py34,py35,pypy
 
 
 [testenv]
@@ -10,11 +10,11 @@ passenv = LANG
 deps =
     webtest
     mock
-    !server421: pytest
+    pytest
     pytest-cov
     pytest-flake8
-    server421: devpi-server==4.2.1
-    server421: pytest<4.1.0
+# BBB,XXX uncomment when devpi-server 5.2.0 is released
+#    server520: devpi-server==5.2.0
 
 
 [testenv:py27-keyfs]