Repository: pypa/twine
Pull request #122 (2015-09-13 04:19:00)

Upload refactor
This properly splits out the logic between repository, package, and command wrappers.

This also adds a `register` command which has been often requested to show off the power of the refactor.

Currently the tests are failing but I can't determine what the difference is between pretend calls via the pytest output. Here's hoping they don't fail here.


Patch:
diff --git a/setup.py b/setup.py
index cfab2e0..ef440ea 100644
--- a/setup.py
+++ b/setup.py
@@ -69,6 +69,7 @@ setup(
     entry_points={
         "twine.registered_commands": [
             "upload = twine.commands.upload:main",
+            "register = twine.commands.register:main",
         ],
         "console_scripts": [
             "twine = twine.__main__:main",
diff --git a/twine/commands/register.py b/twine/commands/register.py
new file mode 100644
index 0000000..67fec6c
--- /dev/null
+++ b/twine/commands/register.py
@@ -0,0 +1,93 @@
+# Copyright 2015 Ian Cordasco
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from __future__ import absolute_import, unicode_literals, print_function
+
+import argparse
+import os.path
+import sys
+
+from twine import exceptions as exc
+from twine.package import PackageFile
+from twine.repository import Repository
+from twine import utils
+
+
+def register(package, repository, username, password, comment, config_file):
+    config = utils.get_repository_from_config(config_file, repository)
+    config["repository"] = utils.normalize_repository_url(
+        config["repository"]
+    )
+
+    print("Registering package to {0}".format(config["repository"]))
+
+    username = utils.get_username(username, config)
+    password = utils.get_password(password, config)
+
+    repository = Repository(config["repository"], username, password)
+
+    if not os.path.exists(package):
+        raise exc.PackageNotFound(
+            '"{0}" does not exist on the file system.'.format(package)
+        )
+
+    resp = repository.register(PackageFile.from_filename(package, comment))
+    repository.close()
+
+    if resp.is_redirect:
+        raise exc.RedirectDetected(
+            ('"{0}" attempted to redirect to "{1}" during upload.'
+             ' Aborting...').format(config["respository"],
+                                    resp.headers["location"]))
+
+    resp.raise_for_status()
+
+
+def main(args):
+    parser = argparse.ArgumentParser(prog="twine register")
+    parser.add_argument(
+        "-r", "--repository",
+        default="pypi",
+        help="The repository to register the package to (default: "
+             "%(default)s)",
+    )
+    parser.add_argument(
+        "-u", "--username",
+        help="The username to authenticate to the repository as",
+    )
+    parser.add_argument(
+        "-p", "--password",
+        help="The password to authenticate to the repository with",
+    )
+    parser.add_argument(
+        "-c", "--comment",
+        help="The comment to include with the distribution file",
+    )
+    parser.add_argument(
+        "--config-file",
+        default="~/.pypirc",
+        help="The .pypirc config file to use",
+    )
+    parser.add_argument(
+        "package",
+        metavar="package",
+        help="File from which we read the package metadata",
+    )
+
+    args = parser.parse_args(args)
+
+    # Call the register function with the args from the command line
+    try:
+        register(**vars(args))
+    except Exception as exc:
+        sys.exit("{exc.__class__.__name__}: {exc}".format(exc=exc))
diff --git a/twine/commands/upload.py b/twine/commands/upload.py
index 710986d..0381479 100644
--- a/twine/commands/upload.py
+++ b/twine/commands/upload.py
@@ -16,42 +16,13 @@ from __future__ import unicode_literals
 
 import argparse
 import glob
-import hashlib
 import os.path
-import subprocess
 import sys
 
-try:
-    from urlparse import urlparse, urlunparse
-except ImportError:
-    from urllib.parse import urlparse, urlunparse
-
-import pkginfo
-import pkg_resources
-import requests
-from requests_toolbelt.multipart import MultipartEncoder
-
 import twine.exceptions as exc
-from twine.utils import get_config, get_username, get_password
-from twine.wheel import Wheel
-from twine.wininst import WinInst
-
-
-DIST_TYPES = {
-    "bdist_wheel": Wheel,
-    "bdist_wininst": WinInst,
-    "bdist_egg": pkginfo.BDist,
-    "sdist": pkginfo.SDist,
-}
-
-DIST_EXTENSIONS = {
-    ".whl": "bdist_wheel",
-    ".exe": "bdist_wininst",
-    ".egg": "bdist_egg",
-    ".tar.bz2": "sdist",
-    ".tar.gz": "sdist",
-    ".zip": "sdist",
-}
+from twine.package import PackageFile
+from twine.repository import Repository
+from twine import utils
 
 
 def group_wheel_files_first(files):
@@ -82,14 +53,6 @@ def find_dists(dists):
     return group_wheel_files_first(uploads)
 
 
-def sign_file(sign_with, filename, identity):
-    print("Signing {0}".format(os.path.basename(filename)))
-    gpg_args = [sign_with, "--detach-sign", "-a", filename]
-    if identity:
-        gpg_args[2:2] = ["--local-user", identity]
-    subprocess.check_call(gpg_args)
-
-
 def upload(dists, repository, sign, identity, username, password, comment,
            sign_with, config_file):
     # Check that a nonsensical option wasn't given
@@ -102,153 +65,39 @@ def upload(dists, repository, sign, identity, username, password, comment,
     )
     dists = [i for i in dists if not i.endswith(".asc")]
 
-    # Get our config from the .pypirc file
-    try:
-        config = get_config(config_file)[repository]
-    except KeyError:
-        msg = (
-            "Missing '{repo}' section from the configuration file.\n"
-            "Maybe you have a out-dated '{cfg}' format?\n"
-            "more info: "
-            "https://docs.python.org/distutils/packageindex.html#pypirc\n"
-        ).format(
-            repo=repository,
-            cfg=config_file
-        )
-        raise KeyError(msg)
+    config = utils.get_repository_from_config(config_file, repository)
 
-    parsed = urlparse(config["repository"])
-    if parsed.netloc in ["pypi.python.org", "testpypi.python.org"]:
-        config["repository"] = urlunparse(
-            ("https",) + parsed[1:]
-        )
+    config["repository"] = utils.normalize_repository_url(
+        config["repository"]
+    )
 
     print("Uploading distributions to {0}".format(config["repository"]))
 
-    username = get_username(username, config)
-    password = get_password(password, config)
+    username = utils.get_username(username, config)
+    password = utils.get_password(password, config)
 
-    session = requests.session()
+    repository = Repository(config["repository"], username, password)
 
     uploads = find_dists(dists)
 
     for filename in uploads:
+        package = PackageFile.from_filename(filename, comment)
         # Sign the dist if requested
-        if sign:
-            sign_file(sign_with, filename, identity)
-
-        # Extract the metadata from the package
-        for ext, dtype in DIST_EXTENSIONS.items():
-            if filename.endswith(ext):
-                meta = DIST_TYPES[dtype](filename)
-                break
-        else:
-            raise ValueError(
-                "Unknown distribution format: '%s'" %
-                os.path.basename(filename)
-            )
-
-        if dtype == "bdist_egg":
-            pkgd = pkg_resources.Distribution.from_filename(filename)
-            py_version = pkgd.py_version
-        elif dtype == "bdist_wheel":
-            py_version = meta.py_version
-        elif dtype == "bdist_wininst":
-            py_version = meta.py_version
-        else:
-            py_version = None
+        # if sign:
+        #     sign_file(sign_with, filename, identity)
 
-        # Fill in the data - send all the meta-data in case we need to
-        # register a new release
-        data = {
-            # action
-            ":action": "file_upload",
-            "protcol_version": "1",
-
-            # identify release
-            "name": pkg_resources.safe_name(meta.name),
-            "version": meta.version,
-
-            # file content
-            "filetype": dtype,
-            "pyversion": py_version,
-
-            # additional meta-data
-            "metadata_version": meta.metadata_version,
-            "summary": meta.summary,
-            "home_page": meta.home_page,
-            "author": meta.author,
-            "author_email": meta.author_email,
-            "maintainer": meta.maintainer,
-            "maintainer_email": meta.maintainer_email,
-            "license": meta.license,
-            "description": meta.description,
-            "keywords": meta.keywords,
-            "platform": meta.platforms,
-            "classifiers": meta.classifiers,
-            "download_url": meta.download_url,
-            "supported_platform": meta.supported_platforms,
-            "comment": comment,
-
-            # PEP 314
-            "provides": meta.provides,
-            "requires": meta.requires,
-            "obsoletes": meta.obsoletes,
-
-            # Metadata 1.2
-            "project_urls": meta.project_urls,
-            "provides_dist": meta.provides_dist,
-            "obsoletes_dist": meta.obsoletes_dist,
-            "requires_dist": meta.requires_dist,
-            "requires_external": meta.requires_external,
-            "requires_python": meta.requires_python,
-
-        }
-
-        md5_hash = hashlib.md5()
-        with open(filename, "rb") as fp:
-            content = fp.read(4096)
-            while content:
-                md5_hash.update(content)
-                content = fp.read(4096)
-
-        data["md5_digest"] = md5_hash.hexdigest()
-
-        signed_name = os.path.basename(filename) + ".asc"
+        # signed_name = os.path.basename(filename) + ".asc"
+        signed_name = package.signed_filename
         if signed_name in signatures:
             with open(signatures[signed_name], "rb") as gpg:
-                data["gpg_signature"] = (signed_name, gpg.read())
+                package.gpg_signature = (signed_name, gpg.read())
+                # data["gpg_signature"] = (signed_name, gpg.read())
         elif sign:
-            with open(filename + ".asc", "rb") as gpg:
-                data["gpg_signature"] = (signed_name, gpg.read())
+            package.sign(sign_with, identity)
+            # with open(filename + ".asc", "rb") as gpg:
+            #     data["gpg_signature"] = (signed_name, gpg.read())
 
-        print("Uploading {0}".format(os.path.basename(filename)))
-
-        data_to_send = []
-        for key, value in data.items():
-            if isinstance(value, (list, tuple)):
-                for item in value:
-                    data_to_send.append((key, item))
-            else:
-                data_to_send.append((key, value))
-
-        with open(filename, "rb") as fp:
-            data_to_send.append((
-                "content",
-                (os.path.basename(filename), fp, "application/octet-stream"),
-            ))
-            encoder = MultipartEncoder(data_to_send)
-            resp = session.post(
-                config["repository"],
-                data=encoder,
-                auth=(username, password),
-                allow_redirects=False,
-                headers={'Content-Type': encoder.content_type},
-            )
-        # Bug 28. Try to silence a ResourceWarning by releasing the socket and
-        # clearing the connection pool.
-        resp.close()
-        session.close()
+        resp = repository.upload(package)
 
         # Bug 92. If we get a redirect we should abort because something seems
         # funky. The behaviour is not well defined and redirects being issued
@@ -262,6 +111,10 @@ def upload(dists, repository, sign, identity, username, password, comment,
         # Otherwise, raise an HTTPError based on the status code.
         resp.raise_for_status()
 
+    # Bug 28. Try to silence a ResourceWarning by clearing the connection
+    # pool.
+    repository.close()
+
 
 def main(args):
     parser = argparse.ArgumentParser(prog="twine upload")
diff --git a/twine/exceptions.py b/twine/exceptions.py
index 52fa6cb..5783a9d 100644
--- a/twine/exceptions.py
+++ b/twine/exceptions.py
@@ -22,3 +22,11 @@ class RedirectDetected(Exception):
     redirecting them.
     """
     pass
+
+
+class PackageNotFound(Exception):
+    """A package file was provided that could not be found on the file system.
+
+    This is only used when attempting to register a package.
+    """
+    pass
diff --git a/twine/package.py b/twine/package.py
new file mode 100644
index 0000000..a3fbae2
--- /dev/null
+++ b/twine/package.py
@@ -0,0 +1,144 @@
+# Copyright 2015 Ian Cordasco
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from __future__ import absolute_import, unicode_literals, print_function
+import hashlib
+import os
+import subprocess
+
+import pkginfo
+import pkg_resources
+
+from twine.wheel import Wheel
+from twine.wininst import WinInst
+
+DIST_TYPES = {
+    "bdist_wheel": Wheel,
+    "bdist_wininst": WinInst,
+    "bdist_egg": pkginfo.BDist,
+    "sdist": pkginfo.SDist,
+}
+
+DIST_EXTENSIONS = {
+    ".whl": "bdist_wheel",
+    ".exe": "bdist_wininst",
+    ".egg": "bdist_egg",
+    ".tar.bz2": "sdist",
+    ".tar.gz": "sdist",
+    ".zip": "sdist",
+}
+
+
+class PackageFile(object):
+    def __init__(self, filename, comment, metadata, python_version, filetype):
+        self.filename = filename
+        self.basefilename = os.path.basename(filename)
+        self.comment = comment
+        self.metadata = metadata
+        self.python_version = python_version
+        self.filetype = filetype
+        self.safe_name = pkg_resources.safe_name(metadata.name)
+        self.signed_filename = self.filename + '.asc'
+        self.gpg_signature = None
+
+        md5_hash = hashlib.md5()
+        with open(filename, "rb") as fp:
+            content = fp.read(4096)
+            while content:
+                md5_hash.update(content)
+                content = fp.read(4096)
+
+        self.md5_digest = md5_hash.hexdigest()
+
+    @classmethod
+    def from_filename(cls, filename, comment):
+        # Extract the metadata from the package
+        for ext, dtype in DIST_EXTENSIONS.items():
+            if filename.endswith(ext):
+                meta = DIST_TYPES[dtype](filename)
+                break
+        else:
+            raise ValueError(
+                "Unknown distribution format: '%s'" %
+                os.path.basename(filename)
+            )
+
+        if dtype == "bdist_egg":
+            pkgd = pkg_resources.Distribution.from_filename(filename)
+            py_version = pkgd.py_version
+        elif dtype == "bdist_wheel":
+            py_version = meta.py_version
+        elif dtype == "bdist_wininst":
+            py_version = meta.py_version
+        else:
+            py_version = None
+
+        return cls(filename, comment, meta, py_version, dtype)
+
+    def metadata_dictionary(self):
+        meta = self.metadata
+        data = {
+            # identify release
+            "name": self.safe_name,
+            "version": meta.version,
+
+            # file content
+            "filetype": self.filetype,
+            "pyversion": self.python_version,
+
+            # additional meta-data
+            "metadata_version": meta.metadata_version,
+            "summary": meta.summary,
+            "home_page": meta.home_page,
+            "author": meta.author,
+            "author_email": meta.author_email,
+            "maintainer": meta.maintainer,
+            "maintainer_email": meta.maintainer_email,
+            "license": meta.license,
+            "description": meta.description,
+            "keywords": meta.keywords,
+            "platform": meta.platforms,
+            "classifiers": meta.classifiers,
+            "download_url": meta.download_url,
+            "supported_platform": meta.supported_platforms,
+            "comment": self.comment,
+            "md5_digest": self.md5_digest,
+
+            # PEP 314
+            "provides": meta.provides,
+            "requires": meta.requires,
+            "obsoletes": meta.obsoletes,
+
+            # Metadata 1.2
+            "project_urls": meta.project_urls,
+            "provides_dist": meta.provides_dist,
+            "obsoletes_dist": meta.obsoletes_dist,
+            "requires_dist": meta.requires_dist,
+            "requires_external": meta.requires_external,
+            "requires_python": meta.requires_python,
+        }
+
+        if self.gpg_signature is not None:
+            data['gpg_signature'] = self.gpg_signature
+
+        return data
+
+    def sign(self, sign_with, identity):
+        print("Signing {0}".format(self.basefilename))
+        gpg_args = (sign_with, "--detach-sign", "-a", self.filename)
+        if identity:
+            gpg_args += ("--local-user", identity)
+        subprocess.check_call(gpg_args)
+
+        with open(self.signed_filename, "rb") as gpg:
+            self.pg_signature = (self.signed_filename, gpg.read())
diff --git a/twine/repository.py b/twine/repository.py
new file mode 100644
index 0000000..dc473aa
--- /dev/null
+++ b/twine/repository.py
@@ -0,0 +1,87 @@
+# Copyright 2015 Ian Cordasco
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from __future__ import absolute_import, unicode_literals, print_function
+
+import requests
+from requests_toolbelt.multipart import MultipartEncoder
+
+
+class Repository(object):
+    def __init__(self, repository_url, username, password):
+        self.url = repository_url
+        self.session = requests.session()
+        self.session.auth = (username, password)
+
+    def close(self):
+        self.session.close()
+
+    @staticmethod
+    def _convert_data_to_list_of_tuples(data):
+        data_to_send = []
+        for key, value in data.items():
+            if isinstance(value, (list, tuple)):
+                for item in value:
+                    data_to_send.append((key, item))
+            else:
+                data_to_send.append((key, value))
+        return data_to_send
+
+    def register(self, package):
+        data = package.metadata_dictionary()
+        data.update({
+            ":action": "submit",
+            "protocol_version": "1",
+        })
+
+        print("Registering {0}".format(package.basefilename))
+
+        data_to_send = self._convert_data_to_list_of_tuples(data)
+        encoder = MultipartEncoder(data_to_send)
+        resp = self.session.post(
+            self.url,
+            data=encoder,
+            allow_redirects=False,
+            headers={'Content-Type': encoder.content_type},
+        )
+        # Bug 28. Try to silence a ResourceWarning by releasing the socket.
+        resp.close()
+        return resp
+
+    def upload(self, package):
+        data = package.metadata_dictionary()
+        data.update({
+            # action
+            ":action": "file_upload",
+            "protcol_version": "1",
+        })
+
+        data_to_send = self._convert_data_to_list_of_tuples(data)
+
+        print("Uploading {0}".format(package.basefilename))
+
+        with open(package.filename, "rb") as fp:
+            data_to_send.append((
+                "content",
+                (package.basefilename, fp, "application/octet-stream"),
+            ))
+            encoder = MultipartEncoder(data_to_send)
+
+            resp = self.session.post(
+                self.url,
+                data=encoder,
+                allow_redirects=False,
+                headers={'Content-Type': encoder.content_type},
+            )
+
+        return resp
diff --git a/twine/utils.py b/twine/utils.py
index b9082fb..db49d14 100644
--- a/twine/utils.py
+++ b/twine/utils.py
@@ -25,6 +25,11 @@ try:
 except ImportError:  # pragma: no cover
     import ConfigParser as configparser
 
+try:
+    from urlparse import urlparse, urlunparse
+except ImportError:
+    from urllib.parse import urlparse, urlunparse
+
 # Shim for raw_input in python3
 if sys.version_info > (3,):
     input_func = input
@@ -87,6 +92,30 @@ def get_config(path="~/.pypirc"):
     return config
 
 
+def get_repository_from_config(config_file, repository):
+    # Get our config from the .pypirc file
+    try:
+        return get_config(config_file)[repository]
+    except KeyError:
+        msg = (
+            "Missing '{repo}' section from the configuration file.\n"
+            "Maybe you have a out-dated '{cfg}' format?\n"
+            "more info: "
+            "https://docs.python.org/distutils/packageindex.html#pypirc\n"
+        ).format(
+            repo=repository,
+            cfg=config_file
+        )
+        raise KeyError(msg)
+
+
+def normalize_repository_url(url):
+    parsed = urlparse(url)
+    if parsed.netloc in ["pypi.python.org", "testpypi.python.org"]:
+        return urlunparse(("https",) + parsed[1:])
+    return urlunparse(parsed)
+
+
 def get_userpass_value(cli_value, config, key, prompt_strategy):
     """Gets the username / password from config.
 


Test patch:
diff --git a/tests/test_package.py b/tests/test_package.py
new file mode 100644
index 0000000..7b15ba9
--- /dev/null
+++ b/tests/test_package.py
@@ -0,0 +1,57 @@
+# Copyright 2015 Ian Cordasco
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from __future__ import unicode_literals
+from twine import package
+
+import pretend
+
+
+def test_sign_file(monkeypatch):
+    replaced_check_call = pretend.call_recorder(lambda args: None)
+    monkeypatch.setattr(package.subprocess, 'check_call', replaced_check_call)
+    filename = 'tests/fixtures/deprecated-pypirc'
+
+    pkg = package.PackageFile(
+        filename=filename,
+        comment=None,
+        metadata=pretend.stub(name="deprecated-pypirc"),
+        python_version=None,
+        filetype=None
+    )
+    try:
+        pkg.sign('gpg2', None)
+    except IOError:
+        pass
+    args = ('gpg2', '--detach-sign', '-a', filename)
+    assert replaced_check_call.calls == [pretend.call(args)]
+
+
+def test_sign_file_with_identity(monkeypatch):
+    replaced_check_call = pretend.call_recorder(lambda args: None)
+    monkeypatch.setattr(package.subprocess, 'check_call', replaced_check_call)
+    filename = 'tests/fixtures/deprecated-pypirc'
+
+    pkg = package.PackageFile(
+        filename=filename,
+        comment=None,
+        metadata=pretend.stub(name="deprecated-pypirc"),
+        python_version=None,
+        filetype=None
+    )
+    try:
+        pkg.sign('gpg', 'identity')
+    except IOError:
+        pass
+    args = ('gpg', '--detach-sign', '-a', filename, '--local-user', 'identity')
+    assert replaced_check_call.calls == [pretend.call(args)]
diff --git a/tests/test_upload.py b/tests/test_upload.py
index a42f5f9..5c843db 100644
--- a/tests/test_upload.py
+++ b/tests/test_upload.py
@@ -16,7 +16,6 @@ from __future__ import unicode_literals
 import os
 import textwrap
 
-import pretend
 import pytest
 
 from twine.commands import upload
@@ -60,25 +59,6 @@ def test_find_dists_handles_real_files():
     assert expected == files
 
 
-def test_sign_file(monkeypatch):
-    replaced_check_call = pretend.call_recorder(lambda args: None)
-    monkeypatch.setattr(upload.subprocess, 'check_call', replaced_check_call)
-
-    upload.sign_file('gpg2', 'my_file.tar.gz', None)
-    args = ['gpg2', '--detach-sign', '-a', 'my_file.tar.gz']
-    assert replaced_check_call.calls == [pretend.call(args)]
-
-
-def test_sign_file_with_identity(monkeypatch):
-    replaced_check_call = pretend.call_recorder(lambda args: None)
-    monkeypatch.setattr(upload.subprocess, 'check_call', replaced_check_call)
-
-    upload.sign_file('gpg', 'my_file.tar.gz', 'identity')
-    args = ['gpg', '--detach-sign', '--local-user', 'identity', '-a',
-            'my_file.tar.gz']
-    assert replaced_check_call.calls == [pretend.call(args)]
-
-
 def test_get_config_old_format(tmpdir):
     pypirc = os.path.join(str(tmpdir), ".pypirc")