From aa4b1994fff5b76dd2205b661d6458cd90bb40ce Mon Sep 17 00:00:00 2001 From: ryneeverett Date: Sun, 27 Oct 2024 22:19:43 -0400 Subject: [PATCH] Document Python API (resolve #791) --- bugwarrior/config/__init__.py | 37 +- bugwarrior/config/data.py | 30 +- bugwarrior/config/schema.py | 10 +- bugwarrior/docs/common_configuration.rst | 2 + bugwarrior/docs/conf.py | 17 +- bugwarrior/docs/getting.rst | 2 - bugwarrior/docs/other-services/api.rst | 17 + .../{third-party.rst => third_party.rst} | 0 bugwarrior/docs/other-services/tutorial.rst | 35 +- bugwarrior/docs/other_services.rst | 3 +- bugwarrior/services/__init__.py | 327 +++++++++++------- bugwarrior/services/deck.py | 2 + bugwarrior/services/youtrack.py | 1 + tests/config/test_data.py | 4 +- tests/test_templates.py | 2 +- 15 files changed, 329 insertions(+), 160 deletions(-) create mode 100644 bugwarrior/docs/other-services/api.rst rename bugwarrior/docs/other-services/{third-party.rst => third_party.rst} (100%) diff --git a/bugwarrior/config/__init__.py b/bugwarrior/config/__init__.py index cd980211..53f00dc7 100644 --- a/bugwarrior/config/__init__.py +++ b/bugwarrior/config/__init__.py @@ -1,23 +1,22 @@ -from .load import BUGWARRIORRC, get_config_path, load_config -from .schema import (ConfigList, - ExpandedPath, - NoSchemeUrl, +""" +Config API +---------- +""" +from .data import BugwarriorData +from .load import BUGWARRIORRC, get_config_path, load_config # noqa: F401 +from .schema import (ConfigList, # noqa: F401 + ExpandedPath, # noqa: F401 + LoggingPath, # noqa: F401 + MainSectionConfig, + NoSchemeUrl, # noqa: F401 ServiceConfig, - StrippedTrailingSlashUrl) -from .secrets import get_keyring, get_service_password + StrippedTrailingSlashUrl, # noqa: F401 + TaskrcPath) # noqa: F401 +from .secrets import get_keyring # noqa: F401 +# NOTE: __all__ determines the stable, public API. __all__ = [ - # load - 'BUGWARRIORRC', - 'get_config_path', - 'load_config', - # schema - 'ConfigList', - 'ExpandedPath', - 'NoSchemeUrl', - 'ServiceConfig', - 'StrippedTrailingSlashUrl', - # secrets - 'get_keyring', - 'get_service_password', + BugwarriorData.__name__, + MainSectionConfig.__name__, + ServiceConfig.__name__, ] diff --git a/bugwarrior/config/data.py b/bugwarrior/config/data.py index 95cd3e60..3698de32 100644 --- a/bugwarrior/config/data.py +++ b/bugwarrior/config/data.py @@ -1,6 +1,7 @@ import json import os import subprocess +import typing from lockfile.pidlockfile import PIDLockFile @@ -28,31 +29,42 @@ def get_data_path(taskrc): class BugwarriorData: + """ Local data storage. + + This exposes taskwarrior's `data.location` configuration value, as well as + an interface to the ``bugwarrior.data`` file which serves as an arbitrary + key-value store. + """ def __init__(self, data_path): - self.datafile = os.path.join(data_path, 'bugwarrior.data') - self.lockfile = os.path.join(data_path, 'bugwarrior-data.lockfile') + self._datafile = os.path.join(data_path, 'bugwarrior.data') + self._lockfile = os.path.join(data_path, 'bugwarrior-data.lockfile') + #: Taskwarrior's ``data.location`` configuration value. If necessary, + #: services can manage their own files here. self.path = data_path - def get_data(self): - with open(self.datafile) as jsondata: + def get_data(self) -> dict: + """ Return all data from the ``bugwarrior.data`` file. """ + with open(self._datafile) as jsondata: return json.load(jsondata) - def get(self, key): + def get(self, key) -> typing.Any: + """ Return a value stored in the ``bugwarrior.data`` file. """ try: return self.get_data()[key] except OSError: # File does not exist. return None def set(self, key, value): - with PIDLockFile(self.lockfile): + """ Set a value in the ``bugwarrior.data`` file. """ + with PIDLockFile(self._lockfile): try: data = self.get_data() except OSError: # File does not exist. - with open(self.datafile, 'w') as jsondata: + with open(self._datafile, 'w') as jsondata: json.dump({key: value}, jsondata) else: - with open(self.datafile, 'w') as jsondata: + with open(self._datafile, 'w') as jsondata: data[key] = value json.dump(data, jsondata) - os.chmod(self.datafile, 0o600) + os.chmod(self._datafile, 0o600) diff --git a/bugwarrior/config/schema.py b/bugwarrior/config/schema.py index f1a44057..8b2d1ffa 100644 --- a/bugwarrior/config/schema.py +++ b/bugwarrior/config/schema.py @@ -102,6 +102,7 @@ class PydanticConfig(pydantic.v1.BaseConfig): class MainSectionConfig(pydantic.v1.BaseModel): + """ The :ref:`common_configuration:Main Section` configuration, plus computed attributes: """ class Config(PydanticConfig): arbitrary_types_allowed = True @@ -110,9 +111,11 @@ class Config(PydanticConfig): targets: ConfigList # added during configuration loading + #: Interactive status. interactive: bool # added during validation (computed field support will land in pydantic-2) + #: Local data storage. data: typing.Optional[BugwarriorData] = None @pydantic.v1.root_validator @@ -290,7 +293,10 @@ def validate_config(config: dict, main_section: str, config_path: str) -> dict: class ServiceConfig(_ServiceConfig): # type: ignore # (dynamic base class) - """ Base class for service configurations. """ + """ Pydantic_ base class for service configurations. + + .. _Pydantic: https://docs.pydantic.dev/latest/ + """ Config = PydanticConfig # Added during validation (computed field support will land in pydantic-2) @@ -327,7 +333,7 @@ def compute_templates(cls, values): project_template = myprojectname - The above would cause all issues to recieve a project name + The above would cause all issues to receive a project name of 'myprojectname', regardless of what the project name of the generated issue was. diff --git a/bugwarrior/docs/common_configuration.rst b/bugwarrior/docs/common_configuration.rst index df35f29c..98fa58fe 100644 --- a/bugwarrior/docs/common_configuration.rst +++ b/bugwarrior/docs/common_configuration.rst @@ -168,6 +168,8 @@ regardless of what project was assigned by the service itself: github.project_template = Office +.. _Password Management: + Password Management ------------------- diff --git a/bugwarrior/docs/conf.py b/bugwarrior/docs/conf.py index 7bb90dc3..ff6f3e72 100644 --- a/bugwarrior/docs/conf.py +++ b/bugwarrior/docs/conf.py @@ -34,6 +34,7 @@ # ones. extensions = [ 'sphinx.ext.autodoc', + 'sphinx.ext.autosectionlabel', 'sphinx.ext.intersphinx', 'sphinx.ext.todo', 'sphinx.ext.coverage', @@ -133,6 +134,17 @@ # If true, keep warnings as "system message" paragraphs in the built documents. #keep_warnings = False +# Show the class constructor arguments under __init__ rather than in the +# header. This way we can omit __init__ when we don't want to document it. +autodoc_class_signature = "separated" + +# Allow duplicate section names across the document collection (e.g., for each +# service). +autosectionlabel_prefix_document = True + +# For autodoc, split parameters onto separate lines when they exceed this +# length. +maximum_signature_line_length = 79 # -- Options for HTML output ---------------------------------------------- @@ -302,4 +314,7 @@ # Example configuration for intersphinx: refer to the Python standard library. -intersphinx_mapping = {'python': ('https://docs.python.org/3', None)} +intersphinx_mapping = { + 'python': ('https://docs.python.org/3', None), + 'requests': ('https://requests.readthedocs.io/en/latest/', None), +} diff --git a/bugwarrior/docs/getting.rst b/bugwarrior/docs/getting.rst index 33308203..32693699 100644 --- a/bugwarrior/docs/getting.rst +++ b/bugwarrior/docs/getting.rst @@ -1,8 +1,6 @@ Getting bugwarrior ================== -.. _requirements: - Requirements ------------ diff --git a/bugwarrior/docs/other-services/api.rst b/bugwarrior/docs/other-services/api.rst new file mode 100644 index 00000000..7361d34e --- /dev/null +++ b/bugwarrior/docs/other-services/api.rst @@ -0,0 +1,17 @@ +Python API +========== + +The interfaces documented here are considered stable. All other interfaces +should be considered private to bugwarrior and are subject to change without +warning, release notes, or semantic version bumping. + +.. automodule:: bugwarrior.services + :members: + :exclude-members: __init__ + :member-order: bysource + +.. automodule:: bugwarrior.config + :members: + :exclude-members: __init__,compute_templates + :imported-members: + :member-order: bysource diff --git a/bugwarrior/docs/other-services/third-party.rst b/bugwarrior/docs/other-services/third_party.rst similarity index 100% rename from bugwarrior/docs/other-services/third-party.rst rename to bugwarrior/docs/other-services/third_party.rst diff --git a/bugwarrior/docs/other-services/tutorial.rst b/bugwarrior/docs/other-services/tutorial.rst index 434d3566..5afa041b 100644 --- a/bugwarrior/docs/other-services/tutorial.rst +++ b/bugwarrior/docs/other-services/tutorial.rst @@ -20,15 +20,25 @@ More likely you'll be writing your own client using an http API, so start off by This example of accessing a local service is quite simple, but you'll likely need to pass additional arguments and perhaps go through a handshake process to authenticate to a remote server. -2. Service File ---------------- +2. Initialize Service +--------------------- -Add a python file with the name of your service in ``bugwarrior/services``. +There are two approaches here, depending on whether your service will be maintained in bugwarrior or will be maintained separately as a :doc:`third party service `. + +If you're sure you're going to be upstreaming your service, clone the bugwarrior repo and create a python file with the name of your service in ``bugwarrior/services``. .. code:: bash touch bugwarrior/services/gitbug.py +If you're going to maintain your service in it's own repository or if you're uncertain if it will be accepted upstream, create a new package for it. + +.. code:: bash + + cd $MY_PROJECTS + mkdir bugwarrior-gitbug + cd bugwarrior-gitbug + touch bugwarrior_gitbug.py 3. Imports ---------- @@ -48,6 +58,8 @@ Fire up your favorite editor and import the base classes and whatever library yo log = logging.getLogger(__name__) +We're going to step through the use of these bugwarrior classes in subsequent sections, but for reference you may find the :doc:`API docs ` helpful. + 4. Configuration Schema ----------------------- @@ -208,15 +220,28 @@ The ``issues`` method is a generator which yields individual issue dictionaries. 7. Service Registration ----------------------- -Add your service class as an ``entry_point`` under the ``[bugwarrior.service]`` section in ``setup.py``. +If you're developing your service in a separate package, it's time to create a ``setup.py`` if you have not done so already, and register the name of your service with the path to your ``Service`` class. .. code:: python - gitbug=bugwarrior.services.gitbug:GitBugService + setup(... + entry_points=""" + [bugwarrior.service] + gitbug=bugwarrior_gitbug:GitBugService + """ + ) + +If you're developing in the bugwarrior repo, you can simply add your entry to the existing ``[bugwarrior.service]`` group. 8. Tests ---------- +.. note:: + + The remainder of this tutorial is not geared towards third-party services. While you are free to use bugwarrior's testing infrastructure, no attempt is being made to maintain the stability of these interfaces at this time. + + + Create a test file and implement at least the minimal service tests by inheriting from ``AbstractServiceTest``. .. code:: bash diff --git a/bugwarrior/docs/other_services.rst b/bugwarrior/docs/other_services.rst index b2556eb7..793af2e2 100644 --- a/bugwarrior/docs/other_services.rst +++ b/bugwarrior/docs/other_services.rst @@ -3,5 +3,6 @@ Other Services .. toctree:: - other-services/third-party.rst + other-services/third_party.rst other-services/tutorial.rst + other-services/api.rst diff --git a/bugwarrior/services/__init__.py b/bugwarrior/services/__init__.py index 3fac1591..23962f8a 100644 --- a/bugwarrior/services/__init__.py +++ b/bugwarrior/services/__init__.py @@ -1,6 +1,11 @@ +""" +Service API +----------- +""" import abc import os import re +import typing from dateutil.parser import parse as parse_date from dateutil.tz import tzlocal @@ -8,6 +13,7 @@ from jinja2 import Template import pytz import requests +import typing_extensions from bugwarrior.config import schema, secrets @@ -58,132 +64,81 @@ def get_processed_url(main_config: schema.MainSectionConfig, url: str): return url -class Service(abc.ABC): - """ Abstract base class for each service """ - # Which class should this service instantiate for holding these issues? - ISSUE_CLASS = None - # Which class defines this service's configuration options? - CONFIG_SCHEMA = None - - def __init__(self, config, main_config): - self.config = config - self.main_config = main_config - - log.info("Working on [%s]", self.config.target) - - def get_password(self, key, login='nousername'): - password = getattr(self.config, key) - keyring_service = self.get_keyring_service(self.config) - if not password or password.startswith("@oracle:"): - password = secrets.get_service_password( - keyring_service, login, oracle=password, - interactive=self.main_config.interactive) - return password - - def get_issue_for_record(self, record, extra=None): - return self.ISSUE_CLASS( - record, self.config, self.main_config, extra=extra) - - def build_annotations(self, annotations, url=None): - final = [] - if url and self.main_config.annotation_links: - final.append(get_processed_url(self.main_config, url)) - if self.main_config.annotation_comments: - for author, message in annotations: - message = message.strip() - if not message or not author: - continue - - if not self.main_config.annotation_newlines: - message = message.replace('\n', '').replace('\r', '') - - annotation_length = self.main_config.annotation_length - if annotation_length: - message = '%s%s' % ( - message[:annotation_length], - '...' if len(message) > annotation_length else '' - ) - final.append('@%s - %s' % (author, message)) - return final - - @abc.abstractmethod - def issues(self): - """ Returns a list of Issue instances representing issues from a remote service. - - Each item in the list should be a dict that looks something like this: - - { - "description": "Some description of the issue", - "project": "some_project", - "priority": "H", - "annotations": [ - "This is an annotation", - "This is another annotation", - ] - } - - - The description can be 'anything' but must be consistent and unique for - issues you're pulling from a remote service. You can and should use - the ``.description(...)`` method to help format your descriptions. - - The project should be a string and may be anything you like. - - The priority should be one of "H", "M", or "L". - """ - raise NotImplementedError() - - @staticmethod - def get_keyring_service(service_config): - """ Given the keyring service name for this service. """ - raise NotImplementedError - - class Issue(abc.ABC): - # Set to a dictionary mapping UDA short names with type and long name. - # - # Example:: - # - # { - # 'project_id': { - # 'type': 'string', - # 'label': 'Project ID', - # }, - # 'ticket_number': { - # 'type': 'number', - # 'label': 'Ticket Number', - # }, - # } - # - # Note: For best results, dictionary keys should be unique! - UDAS = {} - # Should be a tuple of field names (can be UDA names) that are usable for - # uniquely identifying an issue in the foreign system. - UNIQUE_KEY = [] - # Should be a dictionary of value-to-level mappings between the foreign - # system and the string values 'H', 'M' or 'L'. - PRIORITY_MAP = {} - - def __init__(self, foreign_record, config, main_config, extra=None): - self.record = foreign_record - self.config = config - self.main_config = main_config - self.extra = extra if extra else {} + """ Base class for translating from foreign records to taskwarrior tasks. + + The upper case attributes and abstract methods need to be defined by + service implementations, while the lower case attributes and concrete + methods are provided by the base class. + """ + #: Set to a dictionary mapping UDA short names with type and long name. + #: + #: Example:: + #: + #: { + #: 'project_id': { + #: 'type': 'string', + #: 'label': 'Project ID', + #: }, + #: 'ticket_number': { + #: 'type': 'number', + #: 'label': 'Ticket Number', + #: }, + #: } + #: + #: Note: For best results, dictionary keys should be unique! + UDAS: dict + #: Should be a tuple of field names (can be UDA names) that are usable for + #: uniquely identifying an issue in the foreign system. + UNIQUE_KEY: list + #: Should be a dictionary of value-to-level mappings between the foreign + #: system and the string values 'H', 'M' or 'L'. + PRIORITY_MAP: dict + + def __init__(self, + foreign_record: dict, + config: schema.ServiceConfig, + main_config: schema.MainSectionConfig, + extra: dict): + #: Data retrieved from the external service. + self.record: dict = foreign_record + #: An object whose attributes are this service's configuration values. + self.config: schema.ServiceConfig = config + #: An object whose attributes are the + #: :ref:`common_configuration:Main Section` configuration values. + self.main_config: schema.MainSectionConfig = main_config + #: Data computed by the :class:`Service` class. + self.extra: dict = extra @abc.abstractmethod - def to_taskwarrior(self): + def to_taskwarrior(self) -> dict: """ Transform a foreign record into a taskwarrior dictionary.""" raise NotImplementedError() @abc.abstractmethod - def get_default_description(self): + def get_default_description(self) -> str: + """ Return a default description for this task. + + You should probably use :meth:`build_default_description` to achieve + this. + """ raise NotImplementedError() def get_tags_from_labels(self, - labels, + labels: list, toggle_option='import_labels_as_tags', template_option='label_template', - template_variable='label'): + template_variable='label') -> list: + """ Transform labels into suitable taskwarrior tags, respecting configuration options. + + :param `labels`: Returned from the service. + :param `toggle_option`: Option which, if false, would not import labels as tags. + :param `template_option`: Configuration to use as the + :ref:`field template` for each label. + :param `template_variable`: Name to use in the + :ref:`field template` context to refer to the + label. + """ tags = [] if not getattr(self.config, toggle_option): @@ -199,7 +154,9 @@ def get_tags_from_labels(self, return tags - def get_priority(self): + def get_priority(self) -> typing_extensions.Literal['', 'L', 'M', 'H']: + """ Return the priority of this issue, falling back to ``default_priority`` configuration. + """ return self.PRIORITY_MAP.get( self.record.get('priority'), self.config.default_priority @@ -227,7 +184,16 @@ def parse_date(self, date, timezone='UTC'): def build_default_description( self, title='', url='', number='', cls="issue" - ): + ) -> str: + """ Return a default description, respecting configuration options. + + :param `title`: Short description of the task. + :param `url`: URL to the task on the service. + :param `number`: Number associated with the task on the service. + :param `cls`: The abbreviated type of task this is. Preferred options + are ('issue', 'pull_request', 'merge_request', 'todo', 'task', + 'subtask'). + """ cls_markup = { 'issue': 'Is', 'pull_request': 'PR', @@ -248,10 +214,127 @@ def build_default_description( ) +class Service(abc.ABC): + """ Base class for fetching issues from the service. + + The upper case attributes and abstract methods need to be defined by + service implementations, while the lower case attributes and concrete + methods are provided by the base class. + """ + #: Which class should this service instantiate for holding these issues? + ISSUE_CLASS: Issue + #: Which class defines this service's configuration options? + CONFIG_SCHEMA: schema.ServiceConfig + + def __init__(self, config: schema.ServiceConfig, main_config: schema.MainSectionConfig): + #: An object whose attributes are this service's configuration values. + self.config = config + #: An object whose attributes are the + #: :ref:`common_configuration:Main Section` configuration values. + self.main_config = main_config + + log.info("Working on [%s]", self.config.target) + + def get_password(self, key, login='nousername') -> str: + """ Get a secret value, potentially from an :ref:`oracle `. + + The secret key need not be a *password*, per se. + + :param `key`: Name of the configuration field of the given secret. + :param `login`: Username associated with the password in a keyring, if + applicable. + """ + password = getattr(self.config, key) + keyring_service = self.get_keyring_service(self.config) + if not password or password.startswith("@oracle:"): + password = secrets.get_service_password( + keyring_service, login, oracle=password, + interactive=self.main_config.interactive) + return password + + def get_issue_for_record(self, record, extra=None) -> Issue: + """ Instantiate and return an issue for the given record. + + :param `record`: Foreign record. + :param `extra`: Computed data which is not directly from the service. + """ + extra = extra if extra is not None else {} + return self.ISSUE_CLASS( + record, self.config, self.main_config, extra=extra) + + def build_annotations(self, annotations: list, url: typing.Optional[str] = None) -> list: + """ Format annotations, respecting configuration values. + + :param `annotations`: Comments from service. + :param `url`: Url to prepend to the annotations. + """ + final = [] + if url and self.main_config.annotation_links: + final.append(get_processed_url(self.main_config, url)) + if self.main_config.annotation_comments: + for author, message in annotations: + message = message.strip() + if not message or not author: + continue + + if not self.main_config.annotation_newlines: + message = message.replace('\n', '').replace('\r', '') + + annotation_length = self.main_config.annotation_length + if annotation_length: + message = '%s%s' % ( + message[:annotation_length], + '...' if len(message) > annotation_length else '' + ) + final.append('@%s - %s' % (author, message)) + return final + + @abc.abstractmethod + def issues(self): + """ A generator yielding Issue instances representing issues from a remote service. + + Each item in the list should be a dict that looks something like this: + + .. code-block:: python + + { + "description": "Some description of the issue", + "project": "some_project", + "priority": "H", + "annotations": [ + "This is an annotation", + "This is another annotation", + ] + } + + + The description can be 'anything' but must be consistent and unique for + issues you're pulling from a remote service. You can and should use + the ``.description(...)`` method to help format your descriptions. + + The project should be a string and may be anything you like. + + The priority should be one of "H", "M", or "L". + """ + raise NotImplementedError() + + @staticmethod + # @abc.abstractmethod + def get_keyring_service(service_config): + """ Given the keyring service name for this service. """ + raise NotImplementedError + + class Client: - """ Abstract class responsible for making requests to service API's. """ + """ Base class for making requests to service API's. + + This class is not strictly necessary but encourages a well-structured + service in which the details of making and parsing http requests is + compartmentalized. + """ @staticmethod - def json_response(response): + def json_response(response: requests.Response): + """ Return json if response is OK. """ # If we didn't get good results, just bail. if response.status_code != 200: raise OSError( @@ -264,3 +347,11 @@ def json_response(response): else: # Older python-requests return response.json + + +# NOTE: __all__ determines the stable, public API. +__all__ = [ + Client.__name__, + Issue.__name__, + Service.__name__, +] diff --git a/bugwarrior/services/deck.py b/bugwarrior/services/deck.py index 039056b2..a3764c32 100644 --- a/bugwarrior/services/deck.py +++ b/bugwarrior/services/deck.py @@ -120,6 +120,8 @@ class NextcloudDeckIssue(Issue): UNIQUE_KEY = (BOARD_ID, STACK_ID, CARD_ID,) + PRIORITY_MAP = {} # FIXME + def to_taskwarrior(self): return { 'project': self.extra['board']['title'].lower().replace(' ', '_'), diff --git a/bugwarrior/services/youtrack.py b/bugwarrior/services/youtrack.py index 8f9d5145..9dd3a7f9 100644 --- a/bugwarrior/services/youtrack.py +++ b/bugwarrior/services/youtrack.py @@ -77,6 +77,7 @@ class YoutrackIssue(Issue): }, } UNIQUE_KEY = (URL,) + PRIORITY_MAP = {} # FIXME def to_taskwarrior(self): return { diff --git a/tests/config/test_data.py b/tests/config/test_data.py index 70ef1070..19b81bd4 100644 --- a/tests/config/test_data.py +++ b/tests/config/test_data.py @@ -12,13 +12,13 @@ def setUp(self): self.data = data.BugwarriorData(self.lists_path) def assert0600(self): - permissions = oct(os.stat(self.data.datafile).st_mode & 0o777) + permissions = oct(os.stat(self.data._datafile).st_mode & 0o777) # python2 -> 0600, python3 -> 0o600 self.assertIn(permissions, ['0600', '0o600']) def test_get_set(self): # "touch" data file. - with open(self.data.datafile, 'w+') as handle: + with open(self.data._datafile, 'w+') as handle: json.dump({'old': 'stuff'}, handle) self.data.set('key', 'value') diff --git a/tests/test_templates.py b/tests/test_templates.py index ca088928..4ec2c333 100644 --- a/tests/test_templates.py +++ b/tests/test_templates.py @@ -25,7 +25,7 @@ def get_issue( ) main_config = MainSectionConfig(interactive=False, targets=[]) - issue = DumbIssue({}, config, main_config) + issue = DumbIssue({}, config, main_config, {}) issue.to_taskwarrior = lambda: ( self.arbitrary_issue if description is None else description )