Remove the push-port config option

Ports can and should be specified by using proper ssh:// URL forms,
e.g:

    ssh://artifacts@example.com:22200/artifacts

The alternate form of artifacts@example.com:artifacts isn't a valid
URL, and doesn't let you specify a different port. People are used
to this form due to Git continuing to use it but we should encourage
people to use proper URLs.

3 files changed, 10 insertions, 16 deletions

diff --git a/buildstream/_artifactcache/artifactcache.py b/buildstream/_artifactcache/artifactcache.py
index ac4f6080d..7c43b69e2 100644
--- a/buildstream/_artifactcache/artifactcache.py
+++ b/buildstream/_artifactcache/artifactcache.py
@@ -47,24 +47,20 @@ class ArtifactCache():
         artifact_overrides = _yaml.node_get(project_overrides, Mapping, 'artifacts', default_value={})
         override_pull = _yaml.node_get(artifact_overrides, str, 'pull-url', default_value='') or None
         override_push = _yaml.node_get(artifact_overrides, str, 'push-url', default_value='') or None
-        override_push_port = _yaml.node_get(artifact_overrides, int, 'push-port', default_value=22)
 
-        _yaml.node_validate(artifact_overrides, ['pull-url', 'push-url', 'push-port'])
+        _yaml.node_validate(artifact_overrides, ['pull-url', 'push-url'])
 
         if override_pull or override_push:
             self.artifact_pull = override_pull
             self.artifact_push = override_push
-            self.artifact_push_port = override_push_port
 
         elif any((project.artifact_pull, project.artifact_push)):
             self.artifact_pull = project.artifact_pull
             self.artifact_push = project.artifact_push
-            self.artifact_push_port = project.artifact_push_port
 
         else:
             self.artifact_pull = context.artifact_pull
             self.artifact_push = context.artifact_push
-            self.artifact_push_port = context.artifact_push_port
 
         if self.artifact_push:
             if self.artifact_push.startswith("/") or \
diff --git a/buildstream/_artifactcache/ostreecache.py b/buildstream/_artifactcache/ostreecache.py
index 6b2411e7f..af1b27b9e 100644
--- a/buildstream/_artifactcache/ostreecache.py
+++ b/buildstream/_artifactcache/ostreecache.py
@@ -82,8 +82,7 @@ class OSTreeCache(ArtifactCache):
     def preflight(self):
         if self.can_push() and not self.artifact_push.startswith("/"):
             try:
-                pull_url = initialize_push_connection(self.artifact_push,
-                                                      self.artifact_push_port)
+                pull_url = initialize_push_connection(self.artifact_push)
                 if pull_url != self.artifact_pull:
                     raise ArtifactError(
                         "This cache reports its pull URL as {}, but user "
@@ -365,7 +364,6 @@ class OSTreeCache(ArtifactCache):
                     try:
                         pushed = push_artifact(temp_repo.get_path().get_path(),
                                                self.artifact_push,
-                                               self.artifact_push_port,
                                                [ref, weak_ref], output_file)
                     except PushException as e:
                         raise ArtifactError("Failed to push artifact {}: {}".format(ref, e)) from e
diff --git a/buildstream/_artifactcache/pushreceive.py b/buildstream/_artifactcache/pushreceive.py
index 18905fed3..39c92dfdf 100644
--- a/buildstream/_artifactcache/pushreceive.py
+++ b/buildstream/_artifactcache/pushreceive.py
@@ -315,7 +315,7 @@ class PushMessageReader(object):
         return args
 
 
-def parse_remote_location(remotepath, remote_port):
+def parse_remote_location(remotepath):
     """Parse remote artifact cache URL that's been specified in our config."""
     remote_host = remote_user = remote_repo = None
 
@@ -327,7 +327,7 @@ def parse_remote_location(remotepath, remote_port):
         remote_host = url.hostname
         remote_user = url.username
         remote_repo = url.path
-        remote_port = url.port
+        remote_port = url.port or 22
     else:
         # Scp/git style remote (user@hostname:path)
         parts = remotepath.split('@', 1)
@@ -343,6 +343,8 @@ def parse_remote_location(remotepath, remote_port):
                                 'contain a hostname and path separated '
                                 'by ":"'.format(remotepath))
         remote_host, remote_repo = parts
+        # This form doesn't make it possible to specify a non-standard port.
+        remote_port = 22
 
     return remote_host, remote_user, remote_repo, remote_port
 
@@ -634,15 +636,14 @@ class OSTreeReceiver(object):
 #
 # Args:
 #   remote: The ssh remote url to push to
-#   remote_port: The ssh port at the remote url
 #
 # Returns:
 #   (str): The URL that should be used for pushing to this cache.
 #
 # Raises:
 #   PushException if there was an issue connecting to the remote.
-def initialize_push_connection(remote, remote_port):
-    remote_host, remote_user, remote_repo, remote_port = parse_remote_location(remote, remote_port)
+def initialize_push_connection(remote):
+    remote_host, remote_user, remote_repo, remote_port = parse_remote_location(remote)
     ssh_cmd = ssh_commandline(remote_host, remote_user, remote_port)
 
     # We need a short timeout here because if 'remote' isn't reachable at
@@ -684,7 +685,6 @@ def initialize_push_connection(remote, remote_port):
 # Args:
 #   repo: The local repository path
 #   remote: The ssh remote url to push to
-#   remote_port: The ssh port at the remote url
 #   branches: The refs to push
 #   output: The output where logging should go
 #
@@ -695,12 +695,12 @@ def initialize_push_connection(remote, remote_port):
 # Raises:
 #   PushException if there was an error
 #
-def push(repo, remote, remote_port, branches, output):
+def push(repo, remote, branches, output):
 
     logging.basicConfig(format='%(module)s: %(levelname)s: %(message)s',
                         level=logging.INFO, stream=output)
 
-    pusher = OSTreePusher(repo, remote, remote_port, branches, True, False, output=output)
+    pusher = OSTreePusher(repo, remote, branches, True, False, output=output)
 
     def terminate_push():
         pusher.close()