Skip to content

Commit

Permalink
gitlab: Begin migrating owned to default to True
Browse files Browse the repository at this point in the history
Per discussion in GothenburgBitFactory#986, this has a few benefits:

- Our example configuration will actually be valid since it uses
gitlab.com as a host but does not define a filter.
- More similar default behavior to other services, such as github.
- Hopefully less confusion about the filtering requirements to
avoid rate-banning and fewer bug reports caused by rate bans.

We start off by implementing the first step, which is to raise a
warning if 'owned' is undefined. The remainder of the migration plan is
in the validator docstring.
  • Loading branch information
ryneeverett committed Jul 14, 2023
1 parent 54f5396 commit 7a3367e
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
19 changes: 18 additions & 1 deletion bugwarrior/services/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class GitlabConfig(config.ServiceConfig):
include_regex: typing.Optional[typing.Pattern] = None
exclude_regex: typing.Optional[typing.Pattern] = None
membership: bool = False
owned: bool = False
owned: typing.Optional[bool] = None
import_labels_as_tags: bool = False
label_template: str = '{{label}}'
include_merge_requests: typing.Union[bool, typing_extensions.Literal['Undefined']] = 'Undefined'
Expand Down Expand Up @@ -99,6 +99,23 @@ def filter_gitlab_dot_com(cls, values):
"there are too many on gitlab.com to fetch them all.")
return values

@pydantic.validator('owned', always=True)
def require_owned(cls, v):
"""
Migrate 'owned' field from default False to default True.
Migration plan:
- [X] Warning if 'owned' is undefined.
- [ ] Next major version: validation error if 'owned' is undefined.
- [ ] Subsequent major version: 'owned' defaults to True.
"""
if v is None:
log.warning(
"WARNING: Gitlab's 'owned' configuration field should be set "
"explicitly. In a future release, this will be an error.")
v = False
return v


class GitlabClient(ServiceClient):
"""Abstraction of Gitlab API v4"""
Expand Down
1 change: 1 addition & 0 deletions tests/config/test_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def setUp(self):
'host': 'my-git.org',
'login': 'arbitrary_login',
'token': 'arbitrary_token',
'owned': 'false',
}

def test_valid(self):
Expand Down
10 changes: 10 additions & 0 deletions tests/test_gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -599,6 +599,16 @@ def test_body_no_limit(self):
issue = dict(description="A very short issue body. Fixes #42.")
self.assertEqual(issue["description"], self.service.description(issue))

def test_undefined_owned_warning(self):
self.config['myservice'].pop('owned')
self.config['myservice']['membership'] = 'true'
self.validate()
self.assertEqual(len(self.caplog.records), 1)
self.assertIn(
"WARNING: Gitlab's 'owned' configuration field should be set "
"explicitly. In a future release, this will be an error.",
self.caplog.records[0].message)


class TestGitlabIssue(AbstractServiceTest, ServiceTest):
maxDiff = None
Expand Down

0 comments on commit 7a3367e

Please sign in to comment.