D3398: wireproto: move supportedcompengines out of wireproto

[indygreg (Gregory Szorc)]

indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This function is used by both version 1 and version 2. It belongs in
  a common module.
  
  "wireprototypes" may not be the best module name. I may rename it...

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D3398

AFFECTED FILES
  mercurial/wireproto.py
  mercurial/wireprotoserver.py
  mercurial/wireprototypes.py
  mercurial/wireprotov2server.py

CHANGE DETAILS

diff --git a/mercurial/wireprotov2server.py b/mercurial/wireprotov2server.py
--- a/mercurial/wireprotov2server.py
+++ b/mercurial/wireprotov2server.py
@@ -393,7 +393,7 @@
     transports.
     """
     compression = []
-    for engine in wireproto.supportedcompengines(repo.ui, util.SERVERROLE):
+    for engine in wireprototypes.supportedcompengines(repo.ui, util.SERVERROLE):
         compression.append({
             b'name': engine.wireprotosupport().name,
         })
diff --git a/mercurial/wireprototypes.py b/mercurial/wireprototypes.py
--- a/mercurial/wireprototypes.py
+++ b/mercurial/wireprototypes.py
@@ -12,6 +12,10 @@
 from .thirdparty.zope import (
     interface as zi,
 )
+from . import (
+    error,
+    util,
+)
 
 # Names of the SSH protocol implementations.
 SSHV1 = 'ssh-v1'
@@ -319,3 +323,52 @@
             return False
 
         return True
+
+def supportedcompengines(ui, role):
+    """Obtain the list of supported compression engines for a request."""
+    assert role in (util.CLIENTROLE, util.SERVERROLE)
+
+    compengines = util.compengines.supportedwireengines(role)
+
+    # Allow config to override default list and ordering.
+    if role == util.SERVERROLE:
+        configengines = ui.configlist('server', 'compressionengines')
+        config = 'server.compressionengines'
+    else:
+        # This is currently implemented mainly to facilitate testing. In most
+        # cases, the server should be in charge of choosing a compression engine
+        # because a server has the most to lose from a sub-optimal choice. (e.g.
+        # CPU DoS due to an expensive engine or a network DoS due to poor
+        # compression ratio).
+        configengines = ui.configlist('experimental',
+                                      'clientcompressionengines')
+        config = 'experimental.clientcompressionengines'
+
+    # No explicit config. Filter out the ones that aren't supposed to be
+    # advertised and return default ordering.
+    if not configengines:
+        attr = 'serverpriority' if role == util.SERVERROLE else 'clientpriority'
+        return [e for e in compengines
+                if getattr(e.wireprotosupport(), attr) > 0]
+
+    # If compression engines are listed in the config, assume there is a good
+    # reason for it (like server operators wanting to achieve specific
+    # performance characteristics). So fail fast if the config references
+    # unusable compression engines.
+    validnames = set(e.name() for e in compengines)
+    invalidnames = set(e for e in configengines if e not in validnames)
+    if invalidnames:
+        raise error.Abort(_('invalid compression engine defined in %s: %s') %
+                          (config, ', '.join(sorted(invalidnames))))
+
+    compengines = [e for e in compengines if e.name() in configengines]
+    compengines = sorted(compengines,
+                         key=lambda e: configengines.index(e.name()))
+
+    if not compengines:
+        raise error.Abort(_('%s config option does not specify any known '
+                            'compression engines') % config,
+                          hint=_('usable compression engines: %s') %
+                          ', '.sorted(validnames))
+
+    return compengines
diff --git a/mercurial/wireprotoserver.py b/mercurial/wireprotoserver.py
--- a/mercurial/wireprotoserver.py
+++ b/mercurial/wireprotoserver.py
@@ -149,7 +149,8 @@
         # FUTURE advertise minrx and mintx after consulting config option
         caps.append('httpmediatype=0.1rx,0.1tx,0.2tx')
 
-        compengines = wireproto.supportedcompengines(repo.ui, util.SERVERROLE)
+        compengines = wireprototypes.supportedcompengines(repo.ui,
+                                                          util.SERVERROLE)
         if compengines:
             comptypes = ','.join(urlreq.quote(e.wireprotosupport().name)
                                  for e in compengines)
@@ -329,7 +330,7 @@
 
         # Now find an agreed upon compression format.
         compformats = wireproto.clientcompressionsupport(proto)
-        for engine in wireproto.supportedcompengines(ui, util.SERVERROLE):
+        for engine in wireprototypes.supportedcompengines(ui, util.SERVERROLE):
             if engine.wireprotosupport().name in compformats:
                 opts = {}
                 level = ui.configint('server', '%slevel' % engine.name())
diff --git a/mercurial/wireproto.py b/mercurial/wireproto.py
--- a/mercurial/wireproto.py
+++ b/mercurial/wireproto.py
@@ -114,55 +114,6 @@
 
     return ui.configbool('server', 'bundle1')
 
-def supportedcompengines(ui, role):
-    """Obtain the list of supported compression engines for a request."""
-    assert role in (util.CLIENTROLE, util.SERVERROLE)
-
-    compengines = util.compengines.supportedwireengines(role)
-
-    # Allow config to override default list and ordering.
-    if role == util.SERVERROLE:
-        configengines = ui.configlist('server', 'compressionengines')
-        config = 'server.compressionengines'
-    else:
-        # This is currently implemented mainly to facilitate testing. In most
-        # cases, the server should be in charge of choosing a compression engine
-        # because a server has the most to lose from a sub-optimal choice. (e.g.
-        # CPU DoS due to an expensive engine or a network DoS due to poor
-        # compression ratio).
-        configengines = ui.configlist('experimental',
-                                      'clientcompressionengines')
-        config = 'experimental.clientcompressionengines'
-
-    # No explicit config. Filter out the ones that aren't supposed to be
-    # advertised and return default ordering.
-    if not configengines:
-        attr = 'serverpriority' if role == util.SERVERROLE else 'clientpriority'
-        return [e for e in compengines
-                if getattr(e.wireprotosupport(), attr) > 0]
-
-    # If compression engines are listed in the config, assume there is a good
-    # reason for it (like server operators wanting to achieve specific
-    # performance characteristics). So fail fast if the config references
-    # unusable compression engines.
-    validnames = set(e.name() for e in compengines)
-    invalidnames = set(e for e in configengines if e not in validnames)
-    if invalidnames:
-        raise error.Abort(_('invalid compression engine defined in %s: %s') %
-                          (config, ', '.join(sorted(invalidnames))))
-
-    compengines = [e for e in compengines if e.name() in configengines]
-    compengines = sorted(compengines,
-                         key=lambda e: configengines.index(e.name()))
-
-    if not compengines:
-        raise error.Abort(_('%s config option does not specify any known '
-                            'compression engines') % config,
-                          hint=_('usable compression engines: %s') %
-                          ', '.sorted(validnames))
-
-    return compengines
-
 # For version 1 transports.
 commands = wireprototypes.commanddict()
 



To: indygreg, #hg-reviewers
Cc: mercurial-devel