diff --git a/bugwarrior/command.py b/bugwarrior/command.py index 7eccc59d..bb591822 100644 --- a/bugwarrior/command.py +++ b/bugwarrior/command.py @@ -12,7 +12,7 @@ from lockfile.pidlockfile import PIDLockFile from bugwarrior.collect import aggregate_issues -from bugwarrior.config import get_config_path, get_keyring, get_service, load_config +from bugwarrior.config import get_config_path, get_keyring, load_config from bugwarrior.db import get_defined_udas_as_strings, synchronize if TYPE_CHECKING: @@ -162,9 +162,7 @@ def targets() -> Iterator[str]: for service_config in config.service_configs: for value in dict(service_config).values(): if isinstance(value, str) and '@oracle:use_keyring' in value: - yield get_service(service_config.service).get_keyring_service( - service_config - ) + yield service_config.keyring_service @vault.command() diff --git a/bugwarrior/config/__init__.py b/bugwarrior/config/__init__.py index 7c3fd9b0..12914862 100644 --- a/bugwarrior/config/__init__.py +++ b/bugwarrior/config/__init__.py @@ -15,9 +15,9 @@ StrippedTrailingSlashUrl, # noqa: F401 TaskrcPath, # noqa: F401 UnsupportedOption, # noqa: F401 + get_service, # noqa:F401 ) from .secrets import get_keyring # noqa: F401 -from .validation import get_service # noqa:F401 # NOTE: __all__ determines the stable, public API. __all__ = [BugwarriorData.__name__, MainSectionConfig.__name__, ServiceConfig.__name__] diff --git a/bugwarrior/config/schema.py b/bugwarrior/config/schema.py index b9a9cae2..319b64c7 100644 --- a/bugwarrior/config/schema.py +++ b/bugwarrior/config/schema.py @@ -1,9 +1,11 @@ +from functools import cache +from importlib.metadata import entry_points import logging import os from pathlib import Path import re import typing -from typing import Annotated, Any, Literal +from typing import TYPE_CHECKING, Annotated, Any, Literal import pydantic from pydantic import ( @@ -23,6 +25,9 @@ from .data import BugwarriorData, get_data_path +if TYPE_CHECKING: + from bugwarrior.services import Service + log = logging.getLogger(__name__) Priority = Literal['', 'L', 'M', 'H'] @@ -197,6 +202,8 @@ class ServiceConfig(_ServiceConfig): .. _Pydantic: https://docs.pydantic.dev/latest/ """ + KEYRING_SERVICE: typing.ClassVar[str] + # Added before validation (computed field) service: str target: str @@ -211,6 +218,16 @@ class ServiceConfig(_ServiceConfig): add_tags: ConfigList = [] static_fields: ConfigList = [] + @property + def keyring_service(self) -> str: + + service = get_service(self.service) + if service.API_VERSION < 2: + assert hasattr(service, "get_keyring_service") + return service.get_keyring_service(self) # ty: ignore + + return self.KEYRING_SERVICE.format(**self.model_dump()) + @model_validator(mode="before") @classmethod def compute_templates(cls, values: dict[str, Any]) -> dict[str, Any]: @@ -277,3 +294,23 @@ def deprecate_project_name(cls, value: str) -> str: if value != '': log.warning('project_name is deprecated in favor of project_template') return value + + +@cache +def get_service(service_name: str) -> type["Service"]: + try: + (service,) = entry_points(group='bugwarrior.service', name=service_name) + except ValueError as e: + if service_name in [ + 'activecollab', + 'activecollab2', + 'megaplan', + 'teamlab', + 'versionone', + ]: + log.warning(f"The {service_name} service has been removed.") + raise ValueError( + f"Configured service '{service_name}' not found. " + "Is it installed? Or misspelled?" + ) from e + return service.load() diff --git a/bugwarrior/config/validation.py b/bugwarrior/config/validation.py index 35da987c..84beb8bb 100644 --- a/bugwarrior/config/validation.py +++ b/bugwarrior/config/validation.py @@ -1,5 +1,3 @@ -from functools import cache -from importlib.metadata import entry_points import logging import sys from typing import TYPE_CHECKING, Annotated, Any, NoReturn, Union @@ -7,31 +5,17 @@ from pydantic import Field, TypeAdapter, ValidationError from pydantic_core import ErrorDetails -from .schema import BaseConfig, Hooks, MainSectionConfig, Notifications, ServiceConfig +from .schema import ( + BaseConfig, + Hooks, + MainSectionConfig, + Notifications, + ServiceConfig, + get_service, +) if TYPE_CHECKING: ServiceConfigType = ServiceConfig - from bugwarrior.services import Service - - -@cache -def get_service(service_name: str) -> type["Service"]: - try: - (service,) = entry_points(group='bugwarrior.service', name=service_name) - except ValueError as e: - if service_name in [ - 'activecollab', - 'activecollab2', - 'megaplan', - 'teamlab', - 'versionone', - ]: - log.warning(f"The {service_name} service has been removed.") - raise ValueError( - f"Configured service '{service_name}' not found. " - "Is it installed? Or misspelled?" - ) from e - return service.load() log = logging.getLogger(__name__) diff --git a/bugwarrior/docs/other-services/api.rst b/bugwarrior/docs/other-services/api.rst index 28968494..6a1330ef 100644 --- a/bugwarrior/docs/other-services/api.rst +++ b/bugwarrior/docs/other-services/api.rst @@ -1,4 +1,4 @@ -Python API v1.0 +Python API v2.0 =============== The interfaces documented here are considered stable. All other interfaces @@ -15,3 +15,14 @@ warning, release notes, or semantic version bumping. :exclude-members: __init__,compute_templates,model_config :imported-members: :member-order: bysource + +Changelog +--------- + +v2.0 +~~~~ + +- Removed ``Service.get_keyring_service(config)``. Service configurations should + define ``KEYRING_SERVICE`` instead. +- Added ``ServiceConfig.KEYRING_SERVICE`` as a format string for generating the + keyring service identifier from service configuration fields. diff --git a/bugwarrior/docs/other-services/tutorial.rst b/bugwarrior/docs/other-services/tutorial.rst index 8b0caa58..fdfc561d 100644 --- a/bugwarrior/docs/other-services/tutorial.rst +++ b/bugwarrior/docs/other-services/tutorial.rst @@ -75,6 +75,7 @@ Now define an initial configuration schema as follows. Don't worry, we're about class GitbugConfig(config.ServiceConfig): service: typing.Literal['gitbug'] + KEYRING_SERVICE = 'gitbug://{path}' path: pathlib.Path @@ -93,6 +94,8 @@ The ``service`` attribute is how bugwarrior will know to assign a given section The ``path`` is the only particular detail required to access our local git-bug instance. You'll likely need additional details such as a username and token to authenticate to the service. Look at how you accessed the API in step 1 and ask yourself which components need to be configurable. +The ``KEYRING_SERVICE`` attribute is a format string that returns a string identifier for secrets in the keyring. Ideally, this string uniquely identifies a given instance of the service when it is possible to have multiple instances of the service configured. Service configuration values may be referenced by field name, such as ``{path}``. + The ``import_labels_as_tags`` and ``port`` attributes create optional configuration fields to allow customization of bugwarrior behavior. .. note:: @@ -187,7 +190,7 @@ Now for the main service class which bugwarrior will invoke to fetch issues. .. code:: python class GitBugService(Service): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = GitBugIssue CONFIG_SCHEMA = GitBugConfig @@ -199,10 +202,6 @@ Now for the main service class which bugwarrior will invoke to fetch issues. port=self.config.port, annotation_comments=self.main_config.annotation_comments) - @staticmethod - def get_keyring_service(config): - return f'gitbug://{config.path}' - def issues(self): for issue in self.client.get_issues(): comments = issue.pop('comments') @@ -217,11 +216,9 @@ Now for the main service class which bugwarrior will invoke to fetch issues. yield self.get_issue_for_record(issue) -Here we see three required class attributes and two required methods. - -The ``API_VERSION`` is set to the latest, while the other two attributes point to our previously defined classes. +Here we see three required class attributes and one required method. -The ``get_keyring_service`` method returns a string identifier for secrets in the keyring. Ideally, this string uniquely identifies a given instance of the service when it is possible to have multiple instances of the service configured. +The ``API_VERSION`` is set to the latest, while ``ISSUE_CLASS`` and ``CONFIG_SCHEMA`` point to our previously defined classes. The ``issues`` method is a generator which yields individual issue dictionaries. diff --git a/bugwarrior/services/__init__.py b/bugwarrior/services/__init__.py index 031a6bbc..3001a471 100644 --- a/bugwarrior/services/__init__.py +++ b/bugwarrior/services/__init__.py @@ -37,7 +37,7 @@ # spec will cause breakages with older bugwarrior releases. # MINOR versions signal extensions of the spec which enhance future releases of # bugwarrior without breaking past releases. -LATEST_API_VERSION = 1.0 +LATEST_API_VERSION = 2.0 class URLShortener: @@ -283,10 +283,9 @@ def get_secret(self, key: str, login: str = 'nousername') -> str: 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 + self.config.keyring_service, login, oracle=password ) return password @@ -298,7 +297,7 @@ def get_issue_for_record( :param `record`: Foreign record. :param `extra`: Computed data which is not directly from the service. """ - extra = extra if extra is not None else {} + extra = extra or {} return self.ISSUE_CLASS(record, self.config, self.main_config, extra=extra) def build_annotations( @@ -359,12 +358,6 @@ def issues(self) -> Iterator[T_Issue]: """ raise NotImplementedError() - @staticmethod - @abc.abstractmethod - def get_keyring_service(config: schema.ServiceConfig) -> str: - """Return the keyring name for this service.""" - raise NotImplementedError - class Client: """Base class for making requests to service API's. diff --git a/bugwarrior/services/azuredevops.py b/bugwarrior/services/azuredevops.py index c4e9f51c..46cc2597 100644 --- a/bugwarrior/services/azuredevops.py +++ b/bugwarrior/services/azuredevops.py @@ -18,6 +18,7 @@ class AzureDevopsConfig(config.ServiceConfig): service: Literal['azuredevops'] + KEYRING_SERVICE = "azuredevops://{organization}@{host}" PAT: str project: EscapedStr organization: EscapedStr @@ -183,7 +184,7 @@ def get_default_description(self) -> str: class AzureDevopsService(Service[AzureDevopsIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = AzureDevopsIssue CONFIG_SCHEMA = AzureDevopsConfig @@ -259,7 +260,3 @@ def issues(self) -> Iterator[AzureDevopsIssue]: } issue_obj.extra.update(extra) yield issue_obj - - @staticmethod - def get_keyring_service(config: AzureDevopsConfig) -> str: - return f"azuredevops://{config.organization}@{config.host}" diff --git a/bugwarrior/services/bitbucket.py b/bugwarrior/services/bitbucket.py index ed0b734d..9a2ac425 100644 --- a/bugwarrior/services/bitbucket.py +++ b/bugwarrior/services/bitbucket.py @@ -15,6 +15,7 @@ class BitbucketConfig(config.ServiceConfig): filter_merge_requests: Union[bool, Literal['Undefined']] = 'Undefined' service: Literal['bitbucket'] + KEYRING_SERVICE = "bitbucket://{key}/{username}" username: str @@ -80,7 +81,7 @@ def get_default_description(self) -> str: class BitbucketService(Service[BitbucketIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = BitbucketIssue CONFIG_SCHEMA = BitbucketConfig @@ -116,10 +117,6 @@ def __init__( 'headers': {'Authorization': f"Bearer {response['access_token']}"} } - @staticmethod - def get_keyring_service(config: BitbucketConfig) -> str: - return f"bitbucket://{config.key}/{config.username}" - def filter_repos(self, repo_tag: str) -> bool: repo = repo_tag.split('/').pop() diff --git a/bugwarrior/services/bts.py b/bugwarrior/services/bts.py index f23b263b..722eebfd 100644 --- a/bugwarrior/services/bts.py +++ b/bugwarrior/services/bts.py @@ -19,6 +19,7 @@ class BTSConfig(config.ServiceConfig): service: typing.Literal['bts'] + KEYRING_SERVICE = 'bts://' email: pydantic.EmailStr = '' packages: config.ConfigList = [] @@ -103,14 +104,10 @@ def get_priority(self) -> config.Priority: class BTSService(Service[BTSIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = BTSIssue CONFIG_SCHEMA = BTSConfig - @staticmethod - def get_keyring_service(config: BTSConfig) -> str: - return 'bts://' - def _record_for_bug(self, bug: debianbts.Bugreport) -> dict[str, Any]: return { 'number': bug.bug_num, diff --git a/bugwarrior/services/bz.py b/bugwarrior/services/bz.py index d1e98a1c..cc67cd9a 100644 --- a/bugwarrior/services/bz.py +++ b/bugwarrior/services/bz.py @@ -34,6 +34,7 @@ def validate_url(value: str) -> str: class BugzillaConfig(config.ServiceConfig): service: typing.Literal['bugzilla'] + KEYRING_SERVICE = "bugzilla://{username}@{base_uri}" username: str base_uri: OptionalSchemeUrl @@ -118,7 +119,7 @@ def get_default_description(self) -> str: class BugzillaService(Service[BugzillaIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = BugzillaIssue CONFIG_SCHEMA = BugzillaConfig @@ -158,10 +159,6 @@ def __init__( password = self.get_secret('password', self.config.username) self.bz.login(self.config.username, password) - @staticmethod - def get_keyring_service(config: BugzillaConfig) -> str: - return f"bugzilla://{config.username}@{config.base_uri}" - def get_owner(self, issue: dict[str, Any]) -> str: return issue['assigned_to'] diff --git a/bugwarrior/services/clickup.py b/bugwarrior/services/clickup.py index 2e23cd7f..9b827685 100644 --- a/bugwarrior/services/clickup.py +++ b/bugwarrior/services/clickup.py @@ -14,6 +14,7 @@ class ClickupConfig(config.ServiceConfig): service: typing.Literal["clickup"] + KEYRING_SERVICE = "clickup://" token: str team_id: int @@ -120,7 +121,7 @@ def parse_timestamp( class ClickupService(Service[ClickupIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = ClickupIssue CONFIG_SCHEMA = ClickupConfig @@ -130,10 +131,6 @@ def __init__( super().__init__(config, main_config) self.client = ClickupClient(token=self.get_secret('token')) - @staticmethod - def get_keyring_service(config: ClickupConfig) -> str: - return "clickup://" - def is_assigned(self, issue: dict) -> bool: if not self.config.only_if_assigned: return True diff --git a/bugwarrior/services/deck.py b/bugwarrior/services/deck.py index 3b70a46a..36cde53e 100644 --- a/bugwarrior/services/deck.py +++ b/bugwarrior/services/deck.py @@ -14,6 +14,7 @@ class NextcloudDeckConfig(config.ServiceConfig): service: typing.Literal['deck'] + KEYRING_SERVICE = 'deck://{username}@{base_uri}' base_uri: config.StrippedTrailingSlashUrl username: str @@ -126,7 +127,7 @@ def get_default_description(self) -> str: class NextcloudDeckService(Service[NextcloudDeckIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = NextcloudDeckIssue CONFIG_SCHEMA = NextcloudDeckConfig @@ -141,10 +142,6 @@ def __init__( password=self.config.password, ) - @staticmethod - def get_keyring_service(config: NextcloudDeckConfig) -> str: - return f'deck://{config.username}@{config.base_uri}' - def get_owner(self, issue: NextcloudDeckIssue) -> str | None: rec = issue.record if rec.get('assignedUsers'): diff --git a/bugwarrior/services/gerrit.py b/bugwarrior/services/gerrit.py index f81728f5..7965e6a3 100644 --- a/bugwarrior/services/gerrit.py +++ b/bugwarrior/services/gerrit.py @@ -15,6 +15,7 @@ class GerritConfig(config.ServiceConfig): service: typing.Literal['gerrit'] + KEYRING_SERVICE = "gerrit://{base_uri}" base_uri: config.StrippedTrailingSlashUrl username: str password: str @@ -72,7 +73,7 @@ def get_default_description(self) -> str: class GerritService(Service[GerritIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = GerritIssue CONFIG_SCHEMA = GerritConfig @@ -102,10 +103,6 @@ def __init__( self.config.username, self.password ) - @staticmethod - def get_keyring_service(config: GerritConfig) -> str: - return f"gerrit://{config.base_uri}" - def issues(self) -> Iterator[GerritIssue]: # Construct the whole url by hand here, because otherwise requests will # percent-encode the ':' characters, which gerrit doesn't like. diff --git a/bugwarrior/services/gitbug.py b/bugwarrior/services/gitbug.py index 608bc7a4..af6a521b 100644 --- a/bugwarrior/services/gitbug.py +++ b/bugwarrior/services/gitbug.py @@ -16,6 +16,7 @@ class GitBugConfig(config.ServiceConfig): service: Literal['gitbug'] + KEYRING_SERVICE = 'gitbug://{path}' path: config.ExpandedPath @@ -138,7 +139,7 @@ def get_default_description(self) -> str: class GitBugService(Service[GitBugIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = GitBugIssue CONFIG_SCHEMA = GitBugConfig @@ -153,10 +154,6 @@ def __init__( annotation_comments=self.main_config.annotation_comments, ) - @staticmethod - def get_keyring_service(config: GitBugConfig) -> str: - return f'gitbug://{config.path}' - def issues(self) -> Iterator[GitBugIssue]: for issue in self.client.get_issues(): comments = issue.pop('comments') diff --git a/bugwarrior/services/github.py b/bugwarrior/services/github.py index fc589007..fe2a0f7e 100644 --- a/bugwarrior/services/github.py +++ b/bugwarrior/services/github.py @@ -25,6 +25,7 @@ class GithubConfig(config.ServiceConfig): # strictly required service: typing.Literal['github'] + KEYRING_SERVICE = "github://{login}@{host}/{username}" login: str token: str @@ -293,7 +294,7 @@ def get_default_description(self) -> str: class GithubService(Service[GithubIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = GithubIssue CONFIG_SCHEMA = GithubConfig @@ -305,10 +306,6 @@ def __init__( auth = {'token': self.get_secret('token', self.config.login)} self.client = GithubClient(self.config.host, auth) - @staticmethod - def get_keyring_service(config: GithubConfig) -> str: - return f"github://{config.login}@{config.host}/{config.username}" - def get_owned_repo_issues(self, tag: str) -> GithubIssueMap: """Grab all the issues""" issues = {} diff --git a/bugwarrior/services/gitlab.py b/bugwarrior/services/gitlab.py index 3f5fbd49..fac9372d 100644 --- a/bugwarrior/services/gitlab.py +++ b/bugwarrior/services/gitlab.py @@ -29,6 +29,7 @@ class GitlabConfig(config.ServiceConfig): filter_merge_requests: typing.Union[bool, typing.Literal['Undefined']] = 'Undefined' service: typing.Literal['gitlab'] + KEYRING_SERVICE = "gitlab://{login}@{host}" login: str token: str host: config.NoSchemeUrl @@ -517,7 +518,7 @@ def get_default_description(self) -> str: class GitlabService(Service[GitlabIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = GitlabIssue CONFIG_SCHEMA = GitlabConfig @@ -537,10 +538,6 @@ def __init__( ) self.repo_map: dict[int, dict[str, Any]] = {} - @staticmethod - def get_keyring_service(config: GitlabConfig) -> str: - return f"gitlab://{config.login}@{config.host}" - def get_owner(self, issue: GitlabIssueEntry) -> list[str]: return [assignee['username'] for assignee in issue[1]['assignees']] diff --git a/bugwarrior/services/gmail.py b/bugwarrior/services/gmail.py index e67ab305..7d3bd45f 100644 --- a/bugwarrior/services/gmail.py +++ b/bugwarrior/services/gmail.py @@ -24,6 +24,7 @@ class GmailConfig(config.ServiceConfig): service: typing.Literal['gmail'] + KEYRING_SERVICE = 'gmail://{login_name}' client_secret_path: config.ExpandedPath = Path('~/.gmail_client_secret.json') query: str = 'label:Starred' @@ -105,7 +106,7 @@ class GmailService(Service[GmailIssue]): APPLICATION_NAME = 'Bugwarrior Gmail Service' SCOPES = ['https://www.googleapis.com/auth/gmail.readonly'] - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = GmailIssue CONFIG_SCHEMA = GmailConfig AUTHENTICATION_LOCK = multiprocessing.Lock() @@ -126,10 +127,6 @@ def __init__( ) self.gmail_api = self.build_api() - @staticmethod - def get_keyring_service(config: GmailConfig) -> str: - return f'gmail://{config.login_name}' - def build_api(self) -> googleapiclient.discovery.Resource: credentials = self.get_credentials() return googleapiclient.discovery.build( diff --git a/bugwarrior/services/jira.py b/bugwarrior/services/jira.py index 77057003..b7eee0f4 100644 --- a/bugwarrior/services/jira.py +++ b/bugwarrior/services/jira.py @@ -78,6 +78,7 @@ def extract_value(self, fields: dict[str, Any]) -> Any: class JiraConfig(config.ServiceConfig): service: typing.Literal['jira'] + KEYRING_SERVICE = "jira://{username}@{base_uri}" base_uri: config.StrippedTrailingSlashUrl username: str @@ -336,7 +337,7 @@ def get_issue_type(self) -> str: class JiraService(Service[JiraIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = JiraIssue CONFIG_SCHEMA = JiraConfig @@ -388,10 +389,6 @@ def _build_jira_client(self) -> JIRA: return JIRA(options=jira_options, auth=(self.config.username, password)) return JIRA(options=jira_options, basic_auth=(self.config.username, password)) - @staticmethod - def get_keyring_service(config: JiraConfig) -> str: - return f"jira://{config.username}@{config.base_uri}" - def body(self, issue: JiraIssue) -> str | None: body = issue.record.get('fields', {}).get('description') diff --git a/bugwarrior/services/kanboard.py b/bugwarrior/services/kanboard.py index ac93e04a..d68ff8e6 100644 --- a/bugwarrior/services/kanboard.py +++ b/bugwarrior/services/kanboard.py @@ -7,6 +7,7 @@ from urllib.parse import urlparse from kanboard import Client +from pydantic import computed_field from bugwarrior import config from bugwarrior.services import Issue, Service @@ -16,6 +17,7 @@ class KanboardConfig(config.ServiceConfig): service: typing.Literal['kanboard'] + KEYRING_SERVICE = "kanboard://{username}@{url_netloc}" url: config.StrippedTrailingSlashUrl username: str password: str @@ -25,6 +27,11 @@ class KanboardConfig(config.ServiceConfig): only_if_assigned: config.UnsupportedOption[str] = '' also_unassigned: config.UnsupportedOption[bool] = False + @computed_field + @property + def url_netloc(self) -> str: + return urlparse(self.url).netloc + class KanboardIssue(Issue): TASK_ID = "kanboardtaskid" @@ -114,7 +121,7 @@ def _convert_timestamp_from_field(self, field: str) -> datetime.datetime | None: class KanboardService(Service[KanboardIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = KanboardIssue CONFIG_SCHEMA = KanboardConfig @@ -169,8 +176,3 @@ def issues(self) -> Iterator[KanboardIssue]: extra["annotations"] = self.annotations(task, extra["url"]) yield self.get_issue_for_record(task, extra) - - @staticmethod - def get_keyring_service(config: KanboardConfig) -> str: - parsed = urlparse(config.url) - return f"kanboard://{config.username}@{parsed.netloc}" diff --git a/bugwarrior/services/linear.py b/bugwarrior/services/linear.py index 711d3fc8..54cd912b 100644 --- a/bugwarrior/services/linear.py +++ b/bugwarrior/services/linear.py @@ -16,6 +16,7 @@ class LinearConfig(config.ServiceConfig): service: typing.Literal["linear"] + KEYRING_SERVICE = "linear://{host}" api_token: str host: config.StrippedTrailingSlashUrl = "https://api.linear.app/graphql" @@ -127,7 +128,7 @@ def get_default_description(self) -> str: class LinearService(Service[LinearIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = LinearIssue CONFIG_SCHEMA = LinearConfig @@ -199,10 +200,6 @@ def __init__( } """ - @staticmethod - def get_keyring_service(config: LinearConfig) -> str: - return f"linear://{config.host}" - def issues(self) -> Iterator[LinearIssue]: for issue in self.get_issues(): yield self.get_issue_for_record(issue, {}) diff --git a/bugwarrior/services/logseq.py b/bugwarrior/services/logseq.py index 784f6343..096de822 100644 --- a/bugwarrior/services/logseq.py +++ b/bugwarrior/services/logseq.py @@ -15,6 +15,7 @@ class LogseqConfig(config.ServiceConfig): service: typing.Literal["logseq"] + KEYRING_SERVICE = "http://{host}:{port}" host: str = "localhost" port: int = 12315 token: str @@ -323,7 +324,7 @@ def get_default_description(self) -> str: class LogseqService(Service[LogseqIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = LogseqIssue CONFIG_SCHEMA = LogseqConfig @@ -340,10 +341,6 @@ def __init__( filter=filter, ) - @staticmethod - def get_keyring_service(config: LogseqConfig) -> str: - return f"http://{config.host}:{config.port}" - def issues(self) -> Iterator[LogseqIssue]: graph_name = self.client.get_graph_name() for issue in self.client.get_issues(): diff --git a/bugwarrior/services/pagure.py b/bugwarrior/services/pagure.py index 7cdccf75..c43e6e34 100644 --- a/bugwarrior/services/pagure.py +++ b/bugwarrior/services/pagure.py @@ -14,8 +14,8 @@ class PagureConfig(config.ServiceConfig): - # strictly required service: typing.Literal['pagure'] + KEYRING_SERVICE = "pagure://{base_url}" base_url: config.StrippedTrailingSlashUrl # conditionally required @@ -91,7 +91,7 @@ def get_default_description(self) -> str: class PagureService(Service[PagureIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = PagureIssue CONFIG_SCHEMA = PagureConfig diff --git a/bugwarrior/services/phab.py b/bugwarrior/services/phab.py index f433630d..1a057753 100644 --- a/bugwarrior/services/phab.py +++ b/bugwarrior/services/phab.py @@ -14,6 +14,7 @@ class PhabricatorConfig(config.ServiceConfig): service: typing.Literal['phabricator'] + KEYRING_SERVICE = 'phabricator://{keyring_host}' user_phids: config.ConfigList = [] project_phids: config.ConfigList = [] @@ -28,6 +29,11 @@ class PhabricatorConfig(config.ServiceConfig): also_unassigned: config.UnsupportedOption[bool] = False + @pydantic.computed_field + @property + def keyring_host(self) -> str: + return str(self.host) if self.host else '' + class PhabricatorIssue(Issue): TITLE = 'phabricatortitle' @@ -80,7 +86,7 @@ def priority(self) -> config.Priority: class PhabricatorService(Service[PhabricatorIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = PhabricatorIssue CONFIG_SCHEMA = PhabricatorConfig @@ -106,10 +112,6 @@ def __init__( else self.config.only_if_assigned ) - @staticmethod - def get_keyring_service(config: PhabricatorConfig) -> str: - return f'phabricator://{config.host if config.host else ""}' - def tasks(self) -> Iterator[PhabricatorIssue]: # If self.config.user_phids or self.config.project_phids is set, # retrict API calls to user_phids or project_phids to avoid time out diff --git a/bugwarrior/services/pivotaltracker.py b/bugwarrior/services/pivotaltracker.py index d832b913..d79a47ef 100644 --- a/bugwarrior/services/pivotaltracker.py +++ b/bugwarrior/services/pivotaltracker.py @@ -16,6 +16,7 @@ class PivotalTrackerConfig(config.ServiceConfig): service: typing.Literal['pivotaltracker'] + KEYRING_SERVICE = 'pivotaltracker://{user_id}@{host}' user_id: int account_ids: config.ConfigList token: str @@ -110,7 +111,7 @@ def get_default_description(self) -> str: class PivotalTrackerService(Service[PivotalTrackerIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = PivotalTrackerIssue CONFIG_SCHEMA = PivotalTrackerConfig @@ -142,10 +143,6 @@ def __init__( if self.config.only_if_author: self.query += f" requester:{self.config.user_id}" - @staticmethod - def get_keyring_service(config: PivotalTrackerConfig) -> str: - return f'pivotaltracker://{config.user_id}@{config.host}' - def annotations( self, annotations: list[dict[str, Any]], story: dict[str, Any] ) -> list[str]: diff --git a/bugwarrior/services/redmine.py b/bugwarrior/services/redmine.py index b2db760f..d065d0a7 100644 --- a/bugwarrior/services/redmine.py +++ b/bugwarrior/services/redmine.py @@ -18,6 +18,7 @@ class RedMineConfig(config.ServiceConfig): project_name: str = '' service: typing.Literal['redmine'] + KEYRING_SERVICE = "redmine://{login}@{url}/" url: config.StrippedTrailingSlashUrl key: str @@ -205,7 +206,7 @@ def get_default_description(self) -> str: class RedMineService(Service[RedMineIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = RedMineIssue CONFIG_SCHEMA = RedMineConfig @@ -232,10 +233,6 @@ def __init__( self.config.verify_ssl, ) - @staticmethod - def get_keyring_service(config: RedMineConfig) -> str: - return f"redmine://{config.login}@{config.url}/" - def issues(self) -> Iterator[RedMineIssue]: issues = self.client.find_issues( self.config.issue_limit, self.config.query, self.config.only_if_assigned diff --git a/bugwarrior/services/taiga.py b/bugwarrior/services/taiga.py index 4e17bfbc..17ea7d15 100644 --- a/bugwarrior/services/taiga.py +++ b/bugwarrior/services/taiga.py @@ -13,6 +13,7 @@ class TaigaConfig(config.ServiceConfig): service: typing.Literal['taiga'] + KEYRING_SERVICE = "taiga://{base_uri}" base_uri: config.StrippedTrailingSlashUrl auth_token: str @@ -60,7 +61,7 @@ def get_default_description(self) -> str: class TaigaService(Service[TaigaIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = TaigaIssue CONFIG_SCHEMA = TaigaConfig @@ -77,10 +78,6 @@ def __init__( } ) - @staticmethod - def get_keyring_service(config: TaigaConfig) -> str: - return f"taiga://{config.base_uri}" - def _issues( self, userid: int, task_type: str, task_type_plural: str, task_type_short: str ) -> Iterator[TaigaIssue]: diff --git a/bugwarrior/services/teamwork_projects.py b/bugwarrior/services/teamwork_projects.py index f730e3bd..42e6c1e9 100644 --- a/bugwarrior/services/teamwork_projects.py +++ b/bugwarrior/services/teamwork_projects.py @@ -13,6 +13,7 @@ class TeamworkConfig(config.ServiceConfig): service: typing.Literal['teamwork_projects'] + KEYRING_SERVICE = 'teamwork_projects://{host}' host: config.StrippedTrailingSlashUrl token: str @@ -93,7 +94,7 @@ def to_taskwarrior(self) -> dict[str, Any]: class TeamworkService(Service[TeamworkIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = TeamworkIssue CONFIG_SCHEMA = TeamworkConfig @@ -106,10 +107,6 @@ def __init__( self.user_id = user["account"]["userId"] self.name = user["account"]["firstname"] + " " + user["account"]["lastname"] - @staticmethod - def get_keyring_service(config: TeamworkConfig) -> str: - return f'teamwork_projects://{config.host}' - def get_comments(self, issue: dict[str, Any]) -> list[str]: if self.main_config.annotation_comments: if issue.get("comments-count", 0) > 0: diff --git a/bugwarrior/services/todoist.py b/bugwarrior/services/todoist.py index 71b7497d..5f4e44b5 100644 --- a/bugwarrior/services/todoist.py +++ b/bugwarrior/services/todoist.py @@ -16,6 +16,7 @@ class TodoistConfig(config.ServiceConfig): service: typing.Literal["todoist"] + KEYRING_SERVICE = "todoist://" token: str filter: str = "(view all)" import_labels_as_tags: bool = False @@ -182,7 +183,7 @@ def get_default_description(self) -> str: class TodoistService(Service[TodoistIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = TodoistIssue CONFIG_SCHEMA = TodoistConfig @@ -207,10 +208,6 @@ def __init__( self.client = TodoistClient(token=self.token, filter=filter) - @staticmethod - def get_keyring_service(config: TodoistConfig) -> str: - return "todoist://" - def annotations( self, user_index: dict[Any, str], issue: dict[str, Any] ) -> list[str]: diff --git a/bugwarrior/services/trac.py b/bugwarrior/services/trac.py index 07aff3c8..80cb25e6 100644 --- a/bugwarrior/services/trac.py +++ b/bugwarrior/services/trac.py @@ -17,6 +17,7 @@ class TracConfig(config.ServiceConfig): service: typing.Literal['trac'] + KEYRING_SERVICE = "https://{username}@{base_uri}/" base_uri: config.NoSchemeUrl scheme: str = 'https' @@ -78,7 +79,7 @@ def get_priority(self) -> config.Priority: class TracService(Service[TracIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = TracIssue CONFIG_SCHEMA = TracConfig trac: offtrac.TracServer | None @@ -101,10 +102,6 @@ def __init__( else: self.trac = offtrac.TracServer(uri + 'login/xmlrpc') - @staticmethod - def get_keyring_service(config: TracConfig) -> str: - return f"https://{config.username}@{config.base_uri}/" - def annotations(self, issue: dict[str, Any]) -> list[str]: annotations = [] # without offtrac, we can't get issue comments diff --git a/bugwarrior/services/trello.py b/bugwarrior/services/trello.py index 1120679c..cdf8e5cb 100644 --- a/bugwarrior/services/trello.py +++ b/bugwarrior/services/trello.py @@ -18,6 +18,7 @@ class TrelloConfig(config.ServiceConfig): service: typing.Literal['trello'] + KEYRING_SERVICE = "trello://{api_key}@trello.com" api_key: str token: str @@ -86,14 +87,10 @@ def to_taskwarrior(self) -> dict[str, Any]: class TrelloService(Service[TrelloIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = TrelloIssue CONFIG_SCHEMA = TrelloConfig - @staticmethod - def get_keyring_service(config: TrelloConfig) -> str: - return f"trello://{config.api_key}@trello.com" - def issues(self) -> Iterator[TrelloIssue]: """ Returns a list of dicts representing issues from a remote service. diff --git a/bugwarrior/services/youtrack.py b/bugwarrior/services/youtrack.py index a3de6e9a..1a3ececd 100644 --- a/bugwarrior/services/youtrack.py +++ b/bugwarrior/services/youtrack.py @@ -15,6 +15,7 @@ class YoutrackConfig(config.ServiceConfig): service: typing.Literal['youtrack'] + KEYRING_SERVICE = "youtrack://{login}@{host}" host: config.NoSchemeUrl login: str token: str @@ -110,7 +111,7 @@ def get_tags(self) -> list[str]: class YoutrackService(Service[YoutrackIssue]): - API_VERSION = 1.0 + API_VERSION = 2.0 ISSUE_CLASS = YoutrackIssue CONFIG_SCHEMA = YoutrackConfig @@ -130,10 +131,6 @@ def __init__( token = self.get_secret('token', self.config.login) self.session.headers['Authorization'] = f'Bearer {token}' - @staticmethod - def get_keyring_service(config: YoutrackConfig) -> str: - return f"youtrack://{config.login}@{config.host}" - def issues(self) -> Iterator[YoutrackIssue]: params = { 'query': self.config.query, diff --git a/tests/base.py b/tests/base.py index 4cc9eada..08ae8e74 100644 --- a/tests/base.py +++ b/tests/base.py @@ -15,6 +15,7 @@ class DumbConfig(config.ServiceConfig): service: typing.Literal["test"] = "test" + KEYRING_SERVICE = 'test://' import_labels_as_tags: bool = False label_template: str = "{{label}}" @@ -33,14 +34,10 @@ def to_taskwarrior(self): class DumbService(services.Service): - API_VERSION = 1.0 + API_VERSION = services.LATEST_API_VERSION ISSUE_CLASS = DumbIssue CONFIG_SCHEMA = DumbConfig - @staticmethod - def get_keyring_service(_): - raise NotImplementedError - def get_owner(self, _): raise NotImplementedError diff --git a/tests/test_clickup.py b/tests/test_clickup.py index c1965179..8a0d434e 100644 --- a/tests/test_clickup.py +++ b/tests/test_clickup.py @@ -169,9 +169,9 @@ def service(self): service = get_service_instances(conf)[0] return service - def test_get_keyring_service(self): + def test_keyring_service(self): conf = self.validate().service_configs[0] - self.assertEqual(ClickupService.get_keyring_service(conf), 'clickup://') + self.assertEqual(conf.keyring_service, 'clickup://') def test_is_assigned(self): task = self.data.get_task() diff --git a/tests/test_github.py b/tests/test_github.py index a470f693..03689eac 100644 --- a/tests/test_github.py +++ b/tests/test_github.py @@ -269,7 +269,7 @@ def test_overwrite_host(self): def test_keyring_service(self): """Checks that the keyring service name""" service_config = GithubConfig(**self.SERVICE_CONFIG, target="myservice") - keyring_service = GithubService.get_keyring_service(service_config) + keyring_service = service_config.keyring_service self.assertEqual("github://tintin@github.com/milou", keyring_service) def test_keyring_service_host(self): @@ -277,7 +277,7 @@ def test_keyring_service_host(self): service_config = GithubConfig( **{'host': 'github.example.com'}, **self.SERVICE_CONFIG, target="myservice" ) - keyring_service = GithubService.get_keyring_service(service_config) + keyring_service = service_config.keyring_service self.assertEqual("github://tintin@github.example.com/milou", keyring_service) def test_get_repository_from_issue_url__issue(self): diff --git a/tests/test_gitlab.py b/tests/test_gitlab.py index 22a34610..60848fb1 100644 --- a/tests/test_gitlab.py +++ b/tests/test_gitlab.py @@ -531,20 +531,16 @@ def service(self): } return service - def test_get_keyring_service_default_host(self): + def test_keyring_service_default_host(self): conf = self.validate() conf = conf.service_configs[0] - self.assertEqual( - GitlabService.get_keyring_service(conf), 'gitlab://foobar@gitlab.com' - ) + self.assertEqual(conf.keyring_service, 'gitlab://foobar@gitlab.com') - def test_get_keyring_service_custom_host(self): + def test_keyring_service_custom_host(self): self.config['myservice']['host'] = 'my-git.org' conf = self.validate() conf = conf.service_configs[0] - self.assertEqual( - GitlabService.get_keyring_service(conf), 'gitlab://foobar@my-git.org' - ) + self.assertEqual(conf.keyring_service, 'gitlab://foobar@my-git.org') def test_filter_gitlab_dot_com(self): self.config['myservice'].update({'host': 'gitlab.com', 'owned': 'false'}) diff --git a/tests/test_kanboard.py b/tests/test_kanboard.py index 3541b7c4..0cca8460 100644 --- a/tests/test_kanboard.py +++ b/tests/test_kanboard.py @@ -34,14 +34,13 @@ def test_validate_config_no_password(self): self.assertValidationError('[kb]\npassword <- Field required') - def test_get_keyring_service(self): + def test_keyring_service(self): self.config["kb"].update( {"url": "http://example.com/", "username": "myuser", "password": "mypass"} ) service_config = self.validate().service_configs[0] self.assertEqual( - KanboardService.get_keyring_service(service_config), - "kanboard://myuser@example.com", + service_config.keyring_service, "kanboard://myuser@example.com" ) diff --git a/tests/test_service.py b/tests/test_service.py index 7584e99b..a4c09740 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -1,9 +1,11 @@ +import abc import pathlib +from pathlib import Path import re import unittest.mock from bugwarrior import services -from bugwarrior.config import validation +from bugwarrior.config import ServiceConfig, validation from bugwarrior.config.load import format_config from .base import ConfigTest, DumbService @@ -31,7 +33,7 @@ def makeIssue(self): service = self.makeService() return service.get_issue_for_record({}) - def checkArchitecture(self, klass): + def checkArchitecture(self, klass: abc.ABCMeta): """ Bidirectional communication between the base classes and their children has been a source of complication as changes to any part of the @@ -43,11 +45,10 @@ def checkArchitecture(self, klass): appear once. This should ensure that these methods are declared here but not called. """ - with open(services.__file__, 'r') as f: - base = f.read() + base = Path(services.__file__).read_text() for method in klass.__abstractmethods__: - references = re.findall(rf'def {method}\(', base) + references = re.findall(rf'{method}\(', base) self.assertEqual(len(references), 1, references) @@ -100,6 +101,20 @@ def test_api_latest_version(self): self.assertEqual(latest_documented, services.LATEST_API_VERSION) + def test_api_v1_keyring_service_backwards_compatibility(self): + class LegacyService: + API_VERSION = 1.0 + + @staticmethod + def get_keyring_service(config): + return f'legacy://{config.target}' + + service_config = ServiceConfig(service='legacy', target='legacy-target') + with unittest.mock.patch( + 'bugwarrior.config.schema.get_service', lambda _: LegacyService + ): + self.assertEqual(service_config.keyring_service, 'legacy://legacy-target') + class TestIssue(ServiceBase): def test_architecture(self): diff --git a/tests/test_trello.py b/tests/test_trello.py index 74b8812b..6cec79e9 100644 --- a/tests/test_trello.py +++ b/tests/test_trello.py @@ -3,7 +3,7 @@ from bugwarrior.collect import TaskConstructor, get_service_instances from bugwarrior.config.schema import MainSectionConfig -from bugwarrior.services.trello import TrelloConfig, TrelloIssue, TrelloService +from bugwarrior.services.trello import TrelloConfig, TrelloIssue from .base import ConfigTest, ServiceTest @@ -256,5 +256,5 @@ def test_valid_config_no_api_key(self): def test_keyring_service(self): """Checks that the keyring service name""" conf = self.validate() - keyring_service = TrelloService.get_keyring_service(conf.service_configs[0]) + keyring_service = conf.service_configs[0].keyring_service self.assertEqual("trello://XXXX@trello.com", keyring_service) diff --git a/tests/test_youtrak.py b/tests/test_youtrak.py index 1efb273a..6a227eb8 100644 --- a/tests/test_youtrak.py +++ b/tests/test_youtrak.py @@ -14,12 +14,11 @@ def setUp(self): 'myservice': {'service': 'youtrack', 'login': 'foobar', 'token': 'XXXXXX'}, } - def test_get_keyring_service(self): + def test_keyring_service(self): self.config['myservice']['host'] = 'youtrack.example.com' service_config = self.validate().service_configs[0] self.assertEqual( - YoutrackService.get_keyring_service(service_config), - 'youtrack://foobar@youtrack.example.com', + service_config.keyring_service, 'youtrack://foobar@youtrack.example.com' )