Skip to content

Service: kwargs only when sending build statuses #12007

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Feb 19, 2025
Merged

Service: kwargs only when sending build statuses #12007

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8856778
Service: kwargs only when sending build statuses
stsewd Feb 19, 2025
4240fd4
Merge branch 'main' into small-refactor-services-x2
stsewd Feb 19, 2025
1fffd2e
Update tests
stsewd Feb 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions readthedocs/builds/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,9 +416,9 @@ def send_build_status(build_pk, commit, status):

for service in service_class.for_project(build.project):
success = service.send_build_status(
build,
commit,
status,
build=build,
commit=commit,
status=status,
)
if success:
log.debug("Build status report sent correctly.")
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/oauth/services/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def update_webhook(self, project, integration):
"""
raise NotImplementedError

def send_build_status(self, build, commit, status):
def send_build_status(self, *, build, commit, status):
Copy link
Member

Choose a reason for hiding this comment

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

What was the meaning of this *, here? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

It forces the function to always be called with kwargs after the *

"""
Create commit status for project.

Expand Down
2 changes: 1 addition & 1 deletion readthedocs/oauth/services/github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ def update_webhook(self, project, integration):

return (False, resp)

def send_build_status(self, build, commit, status):
def send_build_status(self, *, build, commit, status):
"""
Create GitHub commit status for project.

Expand Down
2 changes: 1 addition & 1 deletion readthedocs/oauth/services/gitlab.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ def update_webhook(self, project, integration):

return (False, resp)

def send_build_status(self, build, commit, status):
def send_build_status(self, *, build, commit, status):
"""
Create GitLab commit status for project.

Expand Down
24 changes: 18 additions & 6 deletions readthedocs/rtd_tests/tests/test_oauth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,7 +305,9 @@ def test_multiple_users_same_repo(self):
def test_send_build_status_successful(self, session, mock_logger):
session.post.return_value.status_code = 201
success = self.service.send_build_status(
self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
build=self.external_build,
commit=self.external_build.commit,
status=BUILD_STATUS_SUCCESS,
)

self.assertTrue(success)
Expand All @@ -319,7 +321,9 @@ def test_send_build_status_successful(self, session, mock_logger):
def test_send_build_status_404_error(self, session, mock_logger):
session.post.return_value.status_code = 404
success = self.service.send_build_status(
self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
build=self.external_build,
commit=self.external_build.commit,
status=BUILD_STATUS_SUCCESS,
)

self.assertFalse(success)
Expand All @@ -333,7 +337,9 @@ def test_send_build_status_404_error(self, session, mock_logger):
def test_send_build_status_value_error(self, session, mock_logger):
session.post.side_effect = ValueError
success = self.service.send_build_status(
self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
build=self.external_build,
commit=self.external_build.commit,
status=BUILD_STATUS_SUCCESS,
)

self.assertFalse(success)
Expand Down Expand Up @@ -1163,7 +1169,9 @@ def test_send_build_status_successful(self, repo_id, session, mock_logger):
repo_id().return_value = "9999"

success = self.service.send_build_status(
self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
build=self.external_build,
commit=self.external_build.commit,
status=BUILD_STATUS_SUCCESS,
)

self.assertTrue(success)
Expand All @@ -1180,7 +1188,9 @@ def test_send_build_status_404_error(self, repo_id, session, mock_logger):
repo_id.return_value = "9999"

success = self.service.send_build_status(
self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
build=self.external_build,
commit=self.external_build.commit,
status=BUILD_STATUS_SUCCESS,
)

self.assertFalse(success)
Expand All @@ -1197,7 +1207,9 @@ def test_send_build_status_value_error(self, repo_id, session, mock_logger):
repo_id().return_value = "9999"

success = self.service.send_build_status(
self.external_build, self.external_build.commit, BUILD_STATUS_SUCCESS
build=self.external_build,
commit=self.external_build.commit,
status=BUILD_STATUS_SUCCESS,
)

self.assertFalse(success)
Expand Down