Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Started documenting the API. #212

Closed
wants to merge 1 commit into from

Conversation

mauritsvanrees
Copy link
Contributor

Also changed the register and upload commands to accept keyword arguments.

This refs issue #194

I started out making the commands more usable as API, although @Sigmavirus said he did not view the commands as an API. It may still be useful. It is the most logical first step someone would try when he currently does:

os.system('twine upload stuff')

and wants to try it in pure Python instead:

from twine.commands.upload import upload
upload([stuff])

It would be nice if that worked, instead of having to supply twelve extra arguments, most of which can be None.

So I changed the main functions of the commands to accept keywords. This makes it easy to use them, without needing to change code when someone adds an another argument to the function in twine.

Then I started documenting that, and also started on documenting what is more meant as an API: the stuff in the exceptions, package and repository modules. To me, these seem more like internals that could change at any given moment, but apparently they are meant to be reasonably stable. Instead, the call signature for the commands can change at a moment's notice.

So after starting to document the 'semi-public' API, I could see better how it would be possible for a utility like zest.releaser to use this API without calling the commands.

So....: at this point this pull request feels more like a basis to have a small discussion on. The question then is: what do we want to keep from this? It can be all, or a selection of these, or none:

  • Changing the commands to accept keyword arguments. If this is accepted, zest.releaser is likely to keep using this way, instead of looking at the more official API.
  • Adding the API docs for exceptions, package, and repository. (An edit would be good, especially by someone who knows how to use the gpg/signing parts, which I don't currently use.)
  • Adding the API docs for the commands. We can choose not to do this, if we don't want those commands as an official API.

What do you think?

Also changed the register and upload commands to accept keyword arguments.
format


API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not belong in the README.

@sigmavirus24
Copy link
Member

I started out making the commands more usable as API, although @Sigmavirus [sic] said he did not view the commands as an API.

Correct, they are not suitable for human consumption, even if the commands have defaults for some of the arguments, a single function with that many parameters will never be suitable for an API.

It would be nice if that worked, instead of having to supply twelve extra arguments, most of which can be None.

It might seem nice in theory but it's not maintainable as an API.

So I changed the main functions of the commands to accept keywords. This makes it easy to use them, without needing to change code when someone adds an another argument to the function in twine.

Please revert those changes.

What do you think?

I think there's a lot of unrelated changes in this pull request which are going to muddy the discussion. I've asked you to specifically pull them out.

I'll reiterate my opinion which is:

  1. The twine.commands submodule should not be used by anyone. It appears you still disagree with this, so if I have to, I will break that intentionally in 1.9 by renaming it.
  2. What you've started documenting regarding twine.repository, twine.package, and twine.exceptions is a good start. That should not live in the README though.
  3. I think the README should only document the most popular command-line options as an illustration. If we are going to document all of the options, we're not going to do it using help text alone. We will adopt an approach similar to flake8.

And I'll reiterate my ask of you once more @mauritsvanrees:

If the intended APIs are missing anything you might need for zest.releaser please let me know. I want to make twine.repository, twine.package, and twine.exceptions usable as an API.

@mauritsvanrees
Copy link
Contributor Author

I started out making the commands more usable as API, although @Sigmavirus [sic] said he did not view the commands as an API.

Sorry for spelling your username incorrectly.

The twine.commands submodule should not be used by anyone. It appears you still disagree with this, so if I have to, I will break that intentionally in 1.9 by renaming it.

Charming.
If you were aiming to be clear at the expense of friendliness, you hit the mark.
Point taken, and no offence taken. I won't be pushing for keyword arguments anymore.

What you've started documenting regarding twine.repository, twine.package, and twine.exceptions is a good start. That should not live in the README though.

Good. I did not see a better spot. There is a docs directory, but I don't see any links to the results of that on the PyPI page.
Feel free to take this text as basis and put it somewhere else.

I think the README should only document the most popular command-line options as an illustration. If we are going to document all of the options, we're not going to do it using help text alone. We will adopt an approach similar to flake8.

The current README gave the impression that the shown command line options were simply the result of the last time someone cared to copy-paste the help output, so to me it made sense to update it.

If the intended APIs are missing anything you might need for zest.releaser please let me know. I want to make twine.repository, twine.package, and twine.exceptions usable as an API.

Since a one-line call to the upload command with one or two keyword arguments for the easy case is out of the question, I'll switch to the intended API, using more lines.

@mauritsvanrees
Copy link
Contributor Author

I'll paste the good part of the API here for easier reference and close the pull request.

API
---

``twine`` is written in Python.  Of course.  So if you work on a
Python tool that uses ``twine`` for uploading or registering, you can
import it.

Note that reading environment variables, for example
``TWINE_REPOSITORY``, is a feature of the command line tool.  the API
does not try to read this.


API package

Create a package object:

.. code-block:: python

from twine.package import PackageFile
package = PackageFile.from_filename(filename, comment)

Here filename is the name of a distribution (usually a wheel or a
source distribution) and comment is a comment to include with the
distribution file. The comment may be None.

You can add a gpg signature:

.. code-block:: python

package.add_gpg_signature(signature_filepath, signature_filename)

You can sign a package:

.. code-block:: python

package.sign(sign_with, identity)

API repository


Define a repository:

.. code-block:: python

    from twine.repository import Repository
    repository = Repository(config["repository"], username, password)
    repository.set_certificate_authority(ca_cert)
    repository.set_client_certificate(client_cert)


Register a package:

.. code-block:: python

    package = PackageFile.from_filename(filename, comment)
    response = repository.register()

Upload a package:

.. code-block:: python

    if repository.package_is_uploaded(package):
        ...
    response = repository.upload(package)
    repository.close()


API exceptions

When things go wrong:

.. code-block:: python

# A redirect was detected that the user needs to resolve.
from twine.exceptions import RedirectDetected

# A package file was provided that could not be found on the file system.
from twine.exceptions import PackageNotFound

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants