Skip to content

Commit

Permalink
feat: collect extra shas for gitlab notif
Browse files Browse the repository at this point in the history
depends on codecov/shared#354

These changes collect extra SHAs that we need to (potentially) notify for GitLab users
that use "merge request result" pipelines and the "merge train" feature.

These changes only _collect_ the extra SHAs. The actual notification will happen later.
  • Loading branch information
giovanni-guidini committed Sep 16, 2024
1 parent 8e96f4d commit b8cc8ce
Show file tree
Hide file tree
Showing 5 changed files with 106 additions and 3 deletions.
2 changes: 1 addition & 1 deletion requirements.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
https://github.com/codecov/opentelem-python/archive/refs/tags/v0.0.4a1.tar.gz#egg=codecovopentelem
https://github.com/codecov/shared/archive/42f83ec717e632c543cc6ceab8fc3cc39eccc5be.tar.gz#egg=shared
https://github.com/codecov/shared/archive/f54ab4f53ce09d47b9c8bfb2b7d1bf39ebee4f97.tar.gz#egg=shared
https://github.com/codecov/test-results-parser/archive/1507de2241601d678e514c08b38426e48bb6d47d.tar.gz#egg=test-results-parser
https://github.com/codecov/timestring/archive/d37ceacc5954dff3b5bd2f887936a98a668dda42.tar.gz#egg=timestring
asgiref>=3.7.2
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,7 @@ sentry-sdk[celery]==2.13.0
# via
# -r requirements.in
# shared
shared @ https://github.com/codecov/shared/archive/42f83ec717e632c543cc6ceab8fc3cc39eccc5be.tar.gz
shared @ https://github.com/codecov/shared/archive/f54ab4f53ce09d47b9c8bfb2b7d1bf39ebee4f97.tar.gz
# via -r requirements.in
six==1.16.0
# via
Expand Down
5 changes: 5 additions & 0 deletions services/comparison/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ class ComparisonContext(object):
test_results_error: TestResultsProcessingError | None = None
gh_app_installation_name: str | None = None
gh_is_using_codecov_commenter: bool = False
# GitLab has a "merge results pipeline" (see https://docs.gitlab.com/ee/ci/pipelines/merged_results_pipelines.html)
# This runs on an "internal" commit that is the merge from the PR HEAD and the target branch. This commit only exists in GitLab.
# We need to send commit statuses to this other commit, to guarantee that the check is not ignored.
# See https://docs.gitlab.com/ee/ci/pipelines/merged_results_pipelines.html#successful-merged-results-pipeline-overrides-a-failed-branch-pipeline
gitlab_extra_shas: set[str] | None = None


class ComparisonProxy(object):
Expand Down
44 changes: 44 additions & 0 deletions tasks/notify.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import asyncio
import logging
from typing import Optional

Expand Down Expand Up @@ -378,6 +379,14 @@ def run_impl_within_lock(
"notifications": None,
"reason": "no_head_report",
}

if commit.repository.service == "gitlab":
gitlab_extra_shas_to_notify = async_to_sync(
self.get_gitlab_extra_shas_to_notify
)(commit, repository_service)
else:
gitlab_extra_shas_to_notify = None

log.info(
"We are going to be sending notifications",
extra=dict(
Expand Down Expand Up @@ -409,6 +418,7 @@ def run_impl_within_lock(
gh_is_using_codecov_commenter=self.is_using_codecov_commenter(
repository_service
),
gitlab_extra_shas_to_notify=gitlab_extra_shas_to_notify,
)
self.log_checkpoint(kwargs, UploadFlow.NOTIFIED)
log.info(
Expand Down Expand Up @@ -529,6 +539,9 @@ def save_patch_totals(self, comparison: ComparisonProxy) -> None:
return
head_commit = comparison.head.commit
db_session = head_commit.get_db_session()
if db_session is None:
log.warning("Failed to save patch_totals. dbsession is None")
return
patch_coverage = async_to_sync(comparison.get_patch_totals)()
if (
comparison.project_coverage_base is not None
Expand Down Expand Up @@ -569,6 +582,35 @@ def save_patch_totals(self, comparison: ComparisonProxy) -> None:
)
compare_commit.patch_totals = minimal_totals(patch_coverage)

@sentry_sdk.trace
async def get_gitlab_extra_shas_to_notify(
self, commit: Commit, repository_service: TorngitBaseAdapter
) -> set[str] | None:
""" "
Fetches extra commit SHAs we should send statuses too for GitLab.
GitLab has a "merge results pipeline" (see https://docs.gitlab.com/ee/ci/pipelines/merged_results_pipelines.html)
This runs on an "internal" commit that is the merge from the PR HEAD and the target branch. This commit only exists in GitLab.
We need to send commit statuses to this other commit, to guarantee that the check is not ignored (see https://docs.gitlab.com/ee/ci/pipelines/merged_results_pipelines.html#successful-merged-results-pipeline-overrides-a-failed-branch-pipeline)
"""
log.info(
"Checking if we need to send notification to more commits",
extra=dict(commit=commit.commitid),
)
report = commit.commit_report(ReportType.COVERAGE)
project_id = commit.repository.service_id
job_ids = [
upload.job_code for upload in report.uploads if upload.job_code is not None
]
tasks = [
repository_service.get_pipeline_details(project_id, job_id)
for job_id in job_ids
]
results = await asyncio.gather(*tasks)
return set(
filter(lambda sha: sha is not None and sha != commit.commitid, results)
)

@sentry_sdk.trace
def submit_third_party_notifications(
self,
Expand All @@ -585,6 +627,7 @@ def submit_third_party_notifications(
test_results_error: bool = False,
installation_name_to_use: str = GITHUB_APP_INSTALLATION_DEFAULT_NAME,
gh_is_using_codecov_commenter: bool = False,
gitlab_extra_shas_to_notify: set[str] | None = None,
):
# base_commit is an "adjusted" base commit; for project coverage, we
# compare a PR head's report against its base's report, or if the base
Expand Down Expand Up @@ -618,6 +661,7 @@ def submit_third_party_notifications(
test_results_error=test_results_error,
gh_app_installation_name=installation_name_to_use,
gh_is_using_codecov_commenter=gh_is_using_codecov_commenter,
gitlab_extra_shas=gitlab_extra_shas_to_notify,
),
)

Expand Down
56 changes: 55 additions & 1 deletion tasks/tests/unit/test_notify_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
from typing import Any
from unittest.mock import MagicMock, call

import httpx
import pytest
import respx
from celery.exceptions import MaxRetriesExceededError, Retry
from freezegun import freeze_time
from shared.celery_config import (
Expand All @@ -15,6 +17,7 @@
TorngitClientGeneralError,
TorngitServer5xxCodeError,
)
from shared.torngit.gitlab import Gitlab
from shared.typings.oauth_token_types import Token
from shared.typings.torngit import GithubInstallationInfo, TorngitInstanceData
from shared.yaml import UserYaml
Expand All @@ -27,6 +30,7 @@
PullFactory,
RepositoryFactory,
)
from database.tests.factories.core import ReportFactory, UploadFactory
from helpers.checkpoint_logger import CheckpointLogger, _kwargs_key
from helpers.checkpoint_logger.flows import UploadFlow
from helpers.exceptions import NoConfiguredAppsAvailable, RepositoryWithoutValidBotError
Expand Down Expand Up @@ -348,6 +352,54 @@ def test_possibly_pin_commit_to_github_app_new_selection(self, mocker, dbsession
mock_refresh_selection.assert_called_with(commit)
mock_set_gh_app_for_commit.assert_called_with(12, commit)

@pytest.mark.asyncio
async def test_get_gitlab_extra_shas(self, dbsession):
commit = CommitFactory(
repository__owner__service="gitlab", repository__service_id=1000
)
dbsession.add(commit)
report = ReportFactory(commit=commit)
dbsession.add(report)
uploads = [UploadFactory(report=report, job_code=i) for i in range(3)]
dbsession.add_all(uploads)
dbsession.flush()
assert len(commit.report.uploads) == 3
with respx.mock:
respx.get("https://gitlab.com/api/v4/projects/1000/jobs/0").mock(
return_value=httpx.Response(
status_code=200,
json={
"pipeline": {
"id": 0,
"project_id": 1000,
"sha": commit.commitid,
}
},
)
)
respx.get("https://gitlab.com/api/v4/projects/1000/jobs/1").mock(
return_value=httpx.Response(
status_code=200,
json={
"pipeline": {
"id": 1,
"project_id": 1000,
"sha": "508c25daba5bbc77d8e7cf3c1917d5859153cfd3",
}
},
)
)
respx.get("https://gitlab.com/api/v4/projects/1000/jobs/2").mock(
return_value=httpx.Response(status_code=400, json={})
)
repository_service = Gitlab(token={"key": "some_token"})
task = NotifyTask()
assert await task.get_gitlab_extra_shas_to_notify(
commit, repository_service
) == {
"508c25daba5bbc77d8e7cf3c1917d5859153cfd3",
}


class TestNotifyTask(object):
def test_simple_call_no_notifications(
Expand Down Expand Up @@ -523,7 +575,9 @@ def test_simple_call_yes_notifications_no_base(
ReportService, "get_existing_report_for_commit", return_value=Report()
)
mocked_fetch_pull.return_value = None
commit = CommitFactory.create(message="", pullid=None)
commit = CommitFactory.create(
message="", pullid=None, repository__owner__service="github"
)
dbsession.add(commit)
dbsession.flush()

Expand Down

0 comments on commit b8cc8ce

Please sign in to comment.