diff --git a/swh/lister/core/lister_transports.py b/swh/lister/core/lister_transports.py --- a/swh/lister/core/lister_transports.py +++ b/swh/lister/core/lister_transports.py @@ -49,6 +49,21 @@ 'User-Agent': 'Software Heritage lister (%s)' % self.lister_version } + def request_instance_credentials(self): + """Returns dictionary of any credentials configuration needed by the + forge instance to list. + + Returns: + dict of credentials per instance lister or {} if none. + + """ + all_creds = self.config.get('credentials') + if not all_creds: + return {} + lister_creds = all_creds.get(self.LISTER_NAME, {}) + creds = lister_creds.get(self.instance, {}) + return creds + def request_uri(self, identifier): """Get the full request URI given the transport_request identifier. @@ -93,9 +108,9 @@ """ params = {} params['headers'] = self.request_headers() or {} - all_creds = self.config['credentials'] - lister_creds = all_creds.get(self.LISTER_NAME, {}) - creds = lister_creds.get(self.instance, {}) + creds = self.request_instance_credentials() + if not creds: + return params auth = random.choice(creds) if creds else None if auth: params['auth'] = (auth['username'], auth['password']) diff --git a/swh/lister/gitlab/lister.py b/swh/lister/gitlab/lister.py --- a/swh/lister/gitlab/lister.py +++ b/swh/lister/gitlab/lister.py @@ -2,7 +2,6 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information -import random import time from urllib3.util import parse_url @@ -28,34 +27,6 @@ self.PATH_TEMPLATE = '%s&per_page=%s' % ( self.PATH_TEMPLATE, per_page) - def request_params(self, identifier): - """Get the full parameters passed to requests given the - transport_request identifier. - - For the gitlab lister, the 'credentials' entries is configured - per instance. For example:: - - - credentials: - - gitlab.com: - - username: user0 - password: - - username: user1 - password: - - ... - - other-gitlab-instance: - ... - - """ - params = { - 'headers': self.request_headers() or {} - } - creds_lister = self.config['credentials'].get(self.instance) - if creds_lister: - auth = random.choice(creds_lister) - if auth: - params['auth'] = (auth['username'], auth['password']) - return params - def uid(self, repo): return '%s/%s' % (self.instance, repo['path_with_namespace']) diff --git a/swh/lister/phabricator/lister.py b/swh/lister/phabricator/lister.py --- a/swh/lister/phabricator/lister.py +++ b/swh/lister/phabricator/lister.py @@ -2,33 +2,68 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information +import logging + import urllib.parse from swh.lister.core.indexing_lister import SWHIndexingHttpLister from swh.lister.phabricator.models import PhabricatorModel from collections import defaultdict +logger = logging.getLogger(__name__) -class PhabricatorLister(SWHIndexingHttpLister): - PATH_TEMPLATE = '&order=oldest&attachments[uris]=1&after=%s' +class PhabricatorLister(SWHIndexingHttpLister): + PATH_TEMPLATE = '?order=oldest&attachments[uris]=1&after=%s' MODEL = PhabricatorModel LISTER_NAME = 'phabricator' - def __init__(self, forge_url, api_token, instance=None, + def __init__(self, forge_url, instance=None, api_token=None, override_config=None): if forge_url.endswith("/"): forge_url = forge_url[:-1] self.forge_url = forge_url - api_endpoint = ('api/diffusion.repository.' - 'search?api.token=%s') % api_token - api_baseurl = '%s/%s' % (forge_url, api_endpoint) + api_baseurl = '%s/api/diffusion.repository.search' % forge_url + self.api_token = api_token if not instance: instance = urllib.parse.urlparse(forge_url).hostname self.instance = instance super().__init__(api_baseurl=api_baseurl, override_config=override_config) + def _build_query_params(self, params, api_token): + """Build query params to include the forge's api token + + Returns: + updated params dict with 'params' entry. + + """ + params.update({'params': {'api.token': api_token}}) + return params + + def request_params(self, identifier): + """Override the default params behavior to retrieve the api token + + Credentials are stored as: + + credentials: + phabricator: + : + - username: + password: + + """ + params = {} + params['headers'] = self.request_headers() or {} + if self.api_token: + return self._build_query_params(params, self.api_token) + instance_creds = self.request_instance_credentials() + if not instance_creds: + raise ValueError( + 'Phabricator forge needs authentication credential to list.') + api_token = instance_creds[0]['password'] + return self._build_query_params(params, api_token) + def request_headers(self): """ (Override) Set requests headers to send when querying the diff --git a/swh/lister/phabricator/tasks.py b/swh/lister/phabricator/tasks.py --- a/swh/lister/phabricator/tasks.py +++ b/swh/lister/phabricator/tasks.py @@ -6,10 +6,10 @@ from swh.lister.phabricator.lister import PhabricatorLister -def new_lister(forge_url='https://forge.softwareheritage.org', api_token='', - instance='swh', **kw): - return PhabricatorLister(forge_url=forge_url, api_token=api_token, - instance=instance, **kw) +def new_lister(forge_url='https://forge.softwareheritage.org', instance='swh', + api_token=None, **kw): + return PhabricatorLister( + forge_url=forge_url, instance=instance, api_token=api_token, **kw) @app.task(name=__name__ + '.IncrementalPhabricatorLister') diff --git a/swh/lister/phabricator/tests/test_tasks.py b/swh/lister/phabricator/tests/test_tasks.py --- a/swh/lister/phabricator/tests/test_tasks.py +++ b/swh/lister/phabricator/tests/test_tasks.py @@ -24,7 +24,7 @@ assert res.successful() lister.assert_called_once_with( - api_token='', forge_url='https://forge.softwareheritage.org', + api_token=None, forge_url='https://forge.softwareheritage.org', instance='swh') lister.db_last_index.assert_called_once_with() lister.run.assert_called_once_with(min_bound=42)