From 2524a3dc32b6b047925ee179e95649229ebd32d2 Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 30 Oct 2024 14:17:47 -0700 Subject: [PATCH 1/4] Adjust validate_repo logic --- api/public/v2/tests/test_api_repo_viewset.py | 4 +-- upload/helpers.py | 4 +-- upload/tests/test_helpers.py | 2 +- upload/tests/test_upload.py | 32 ++++++++++---------- upload/tests/views/test_commits.py | 29 ++++++++++++++++++ upload/tests/views/test_reports.py | 31 +++++++++++++++++++ upload/tests/views/test_uploads.py | 2 +- upload/views/commits.py | 3 +- upload/views/reports.py | 3 +- 9 files changed, 86 insertions(+), 24 deletions(-) diff --git a/api/public/v2/tests/test_api_repo_viewset.py b/api/public/v2/tests/test_api_repo_viewset.py index 10579207c8..790078d18b 100644 --- a/api/public/v2/tests/test_api_repo_viewset.py +++ b/api/public/v2/tests/test_api_repo_viewset.py @@ -62,7 +62,7 @@ def test_list(self): "language": self.repo.language, "branch": "main", "active": False, - "activated": False, + "activated": True, "totals": { "branches": 0, "complexity": 0.0, @@ -115,7 +115,7 @@ def test_retrieve(self, get_repo_permissions): "language": self.repo.language, "branch": "main", "active": False, - "activated": False, + "activated": True, "totals": { "branches": 0, "complexity": 0.0, diff --git a/upload/helpers.py b/upload/helpers.py index 77194a8595..c0bd668caf 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -756,10 +756,10 @@ def dispatch_upload_task( def validate_activated_repo(repository): - if repository.active and not repository.activated: + if not repository.activated: settings_url = f"{settings.CODECOV_DASHBOARD_URL}/{repository.author.service}/{repository.author.username}/{repository.name}/settings" raise ValidationError( - f"This repository has been deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings_url}" + f"This repository is deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings_url}" ) diff --git a/upload/tests/test_helpers.py b/upload/tests/test_helpers.py index 053f97ef71..85f7ed52ed 100644 --- a/upload/tests/test_helpers.py +++ b/upload/tests/test_helpers.py @@ -264,7 +264,7 @@ def test_deactivated_repo(db, mocker): with pytest.raises(ValidationError) as exp: validate_activated_repo(repository) assert exp.match( - f"This repository has been deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings_url}" + f"This repository is deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings_url}" ) diff --git a/upload/tests/test_upload.py b/upload/tests/test_upload.py index 56209c855f..cd753bf3aa 100644 --- a/upload/tests/test_upload.py +++ b/upload/tests/test_upload.py @@ -298,7 +298,7 @@ def test_get_global_tokens(self, mock_get_config): def test_determine_repo_upload(self): with self.subTest("token found"): org = G(Owner) - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) params = { "version": "v4", @@ -310,7 +310,7 @@ def test_determine_repo_upload(self): with self.subTest("token not found"): org = G(Owner) - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) params = { "version": "v4", @@ -330,7 +330,7 @@ def test_determine_repo_upload(self): @patch.object(requests, "get") def test_determine_repo_upload_tokenless(self, mock_get): org = G(Owner, username="codecov", service="github") - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) expected_response = { "id": 732059764, "finishTime": f"{datetime.now()}", @@ -503,7 +503,7 @@ def test_determine_upload_commit_to_use( service="bitbucket", oauth_token=encryptor.encode("hahahahaha").decode(), ) - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) upload_params = { "service": "bitbucket", "commit": "3be5c52bd748c508a7e96993c02cf3518c816e84", @@ -519,7 +519,7 @@ def test_determine_upload_commit_to_use( service="github", oauth_token=encryptor.encode("hahahahaha").decode(), ) - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -532,7 +532,7 @@ def test_determine_upload_commit_to_use( with self.subTest("just no bot available"): org = G(Owner, service="github", oauth_token=None) - repo = G(Repository, author=org, private=True) + repo = G(Repository, author=org, private=True, activated=True) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -549,7 +549,7 @@ def test_determine_upload_commit_to_use( service="github", oauth_token=encryptor.encode("hahahahaha").decode(), ) - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -564,7 +564,7 @@ def test_determine_upload_commit_to_use( with self.subTest("use repo bot token when available"): bot = OwnerFactory() org = G(Owner, service="github") - repo = G(Repository, author=org, bot=bot) + repo = G(Repository, author=org, bot=bot, activated=True) upload_params = { "service": "github", @@ -597,7 +597,7 @@ def test_determine_upload_commit_to_use( with self.subTest("HTTP error"): org = G(Owner, service="github") - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -610,7 +610,7 @@ def test_determine_upload_commit_to_use( def test_insert_commit(self): org = G(Owner) - repo = G(Repository, author=org) + repo = G(Repository, author=org, activated=True) with self.subTest("newly created"): insert_commit( @@ -743,7 +743,7 @@ def test_store_report_in_redis(self): def test_validate_upload_repository_moved(self): redis = MockRedis() owner = G(Owner, plan="users-free") - repo = G(Repository, author=owner, name="") + repo = G(Repository, author=owner, name="", activated=True) commit = G(Commit) with self.assertRaises(ValidationError) as err: @@ -757,7 +757,7 @@ def test_validate_upload_repository_moved(self): def test_validate_upload_empty_totals(self): redis = MockRedis() owner = G(Owner, plan="5m") - repo = G(Repository, author=owner) + repo = G(Repository, author=owner, activated=True) commit = G(Commit, totals=None, repository=repo) validate_upload({"commit": commit.commitid}, repo, redis) @@ -769,7 +769,7 @@ def test_validate_upload_empty_totals(self): def test_validate_upload_too_many_uploads_for_commit(self): redis = MockRedis() owner = G(Owner, plan="users-free") - repo = G(Repository, author=owner) + repo = G(Repository, author=owner, activated=True) commit = G(Commit, totals={"s": 151}, repository=repo) report = CommitReportFactory.create(commit=commit) for i in range(151): @@ -782,7 +782,7 @@ def test_validate_upload_too_many_uploads_for_commit(self): def test_validate_upload_repository_blacklisted(self): redis = MockRedis(blacklisted=True) owner = G(Owner, plan="users-free") - repo = G(Repository, author=owner) + repo = G(Repository, author=owner, activated=True) commit = G(Commit) with self.assertRaises(ValidationError) as err: @@ -795,7 +795,7 @@ def test_validate_upload_repository_blacklisted(self): def test_validate_upload_per_repo_billing_invalid(self): redis = MockRedis() owner = G(Owner, plan="1m") - G(Repository, author=owner, private=True, activated=True, active=True) + G(Repository, author=owner, private=True, activated=False, active=True) repo = G(Repository, author=owner, private=True, activated=False, active=False) commit = G(Commit) @@ -876,7 +876,7 @@ def test_validate_upload_valid_upload_repo_activated(self): @freeze_time("2023-01-01T00:00:00") @patch("services.task.TaskService.upload") def test_dispatch_upload_task(self, upload): - repo = G(Repository) + repo = G(Repository, activated=True) task_arguments = { "commit": "commit123", "version": "v4", diff --git a/upload/tests/views/test_commits.py b/upload/tests/views/test_commits.py index e54a8e2327..6b8ae5f415 100644 --- a/upload/tests/views/test_commits.py +++ b/upload/tests/views/test_commits.py @@ -1,6 +1,7 @@ from unittest.mock import patch import pytest +from django.conf import settings from django.urls import reverse from rest_framework.exceptions import ValidationError from rest_framework.test import APIClient @@ -41,6 +42,34 @@ def test_get_repo_not_found(db): upload_views.get_repo() assert exp.match("Repository not found") +def test_deactivated_repo(db): + repo = RepositoryFactory( + name="the_repo", + author__username="codecov", + author__service="github", + active=True, + activated=False, + ) + repo.save() + repo_slug = f"{repo.author.username}::::{repo.name}" + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION="token " + repo.upload_token) + url = reverse( + "new_upload.commits", + args=[repo.author.service, repo_slug], + ) + response = client.post( + url, + {"commitid": "commit_sha"}, + format="json", + ) + response_json = response.json() + assert response.status_code == 400 + assert response_json == [ + f"This repository is deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings.CODECOV_DASHBOARD_URL}/github/codecov/the_repo/settings" + ] + def test_get_queryset(db): target_repo = RepositoryFactory( diff --git a/upload/tests/views/test_reports.py b/upload/tests/views/test_reports.py index 213b2be2f0..ac6d36c245 100644 --- a/upload/tests/views/test_reports.py +++ b/upload/tests/views/test_reports.py @@ -1,6 +1,7 @@ from unittest.mock import patch import pytest +from django.conf import settings from django.urls import reverse from rest_framework.test import APIClient from shared.django_apps.core.tests.factories import ( @@ -38,6 +39,36 @@ def test_reports_get_not_allowed(client, mocker, db): res = client.get(url, **headers) assert res.status_code == 405 +def test_deactivated_repo(db): + repo = RepositoryFactory( + name="the_repo", + author__username="codecov", + author__service="github", + active=True, + activated=False, + ) + commit = CommitFactory(repository=repo) + repo.save() + commit.save() + repo_slug = f"{repo.author.username}::::{repo.name}" + + client = APIClient() + client.credentials(HTTP_AUTHORIZATION="token " + repo.upload_token) + url = reverse( + "new_upload.reports", + args=["github", repo_slug, commit.commitid], + ) + response = client.post( + url, + data={"code": "code1"}, + headers={"User-Agent": "codecov-cli/0.4.7"} + ) + response_json = response.json() + assert response.status_code == 400 + assert response_json == [ + f"This repository is deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings.CODECOV_DASHBOARD_URL}/github/codecov/the_repo/settings" + ] + def test_reports_post(client, db, mocker): mocked_call = mocker.patch.object(TaskService, "preprocess_upload") diff --git a/upload/tests/views/test_uploads.py b/upload/tests/views/test_uploads.py index 9f00230e01..ba08eccbe9 100644 --- a/upload/tests/views/test_uploads.py +++ b/upload/tests/views/test_uploads.py @@ -807,7 +807,7 @@ def test_deactivated_repo(db, mocker, mock_redis): response_json = response.json() assert response.status_code == 400 assert response_json == [ - f"This repository has been deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings.CODECOV_DASHBOARD_URL}/github/codecov/the_repo/settings" + f"This repository is deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings.CODECOV_DASHBOARD_URL}/github/codecov/the_repo/settings" ] diff --git a/upload/views/commits.py b/upload/views/commits.py index cac17e0c33..6c1eb46092 100644 --- a/upload/views/commits.py +++ b/upload/views/commits.py @@ -14,7 +14,7 @@ repo_auth_custom_exception_handler, ) from core.models import Commit -from upload.helpers import generate_upload_sentry_metrics_tags +from upload.helpers import generate_upload_sentry_metrics_tags, validate_activated_repo from upload.serializers import CommitSerializer from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -64,6 +64,7 @@ def perform_create(self, serializer): ), ) repository = self.get_repo() + validate_activated_repo(repository) commit = serializer.save(repository=repository) diff --git a/upload/views/reports.py b/upload/views/reports.py index a0020cbf9a..b471ccdd67 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -16,7 +16,7 @@ ) from reports.models import CommitReport, ReportResults from services.task import TaskService -from upload.helpers import generate_upload_sentry_metrics_tags +from upload.helpers import generate_upload_sentry_metrics_tags, validate_activated_repo from upload.serializers import CommitReportSerializer, ReportResultsSerializer from upload.views.base import GetterMixin from upload.views.uploads import CanDoCoverageUploadsPermission @@ -51,6 +51,7 @@ def perform_create(self, serializer): ), ) repository = self.get_repo() + validate_activated_repo(repository) commit = self.get_commit(repository) log.info( "Request to create new report", From 91f7f55a504de818cf9001e070f6887120b14746 Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 30 Oct 2024 14:29:12 -0700 Subject: [PATCH 2/4] add tests --- upload/helpers.py | 29 +++++++++------------------ upload/tests/test_upload.py | 32 +++++++++++++++--------------- upload/tests/views/test_commits.py | 1 + upload/tests/views/test_reports.py | 5 ++--- upload/views/commits.py | 5 ++++- upload/views/reports.py | 5 ++++- 6 files changed, 36 insertions(+), 41 deletions(-) diff --git a/upload/helpers.py b/upload/helpers.py index 0563f91ccf..7384a1e325 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -756,7 +756,7 @@ def dispatch_upload_task( def validate_activated_repo(repository): - if not repository.activated: + if repository.active and not repository.activated: settings_url = f"{settings.CODECOV_DASHBOARD_URL}/{repository.author.service}/{repository.author.username}/{repository.name}/settings" raise ValidationError( f"This repository is deactivated. To resume uploading to it, please activate the repository in the codecov UI: {settings_url}" @@ -790,7 +790,7 @@ def get_version_from_headers(headers): return "unknown-user-agent" -def generate_upload_prometheus_metrics_labels( +def generate_upload_sentry_metrics_tags( action, request, is_shelter_request, @@ -798,7 +798,6 @@ def generate_upload_prometheus_metrics_labels( repository: Optional[Repository] = None, position: Optional[str] = None, upload_version: Optional[str] = None, - include_empty_labels: bool = True, ): metrics_tags = dict( agent=get_agent_from_headers(request.headers), @@ -807,23 +806,13 @@ def generate_upload_prometheus_metrics_labels( endpoint=endpoint, is_using_shelter="yes" if is_shelter_request else "no", ) - - repo_visibility = None if repository: - repo_visibility = "private" if repository.private else "public" - - optional_fields = { - "repo_visibility": repo_visibility, - "position": position, - "upload_version": upload_version, - } - - metrics_tags.update( - { - field: value - for field, value in optional_fields.items() - if value or include_empty_labels - } - ) + metrics_tags["repo_visibility"] = ( + "private" if repository.private is True else "public" + ) + if position: + metrics_tags["position"] = position + if upload_version: + metrics_tags["upload_version"] = upload_version return metrics_tags diff --git a/upload/tests/test_upload.py b/upload/tests/test_upload.py index cd753bf3aa..56209c855f 100644 --- a/upload/tests/test_upload.py +++ b/upload/tests/test_upload.py @@ -298,7 +298,7 @@ def test_get_global_tokens(self, mock_get_config): def test_determine_repo_upload(self): with self.subTest("token found"): org = G(Owner) - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) params = { "version": "v4", @@ -310,7 +310,7 @@ def test_determine_repo_upload(self): with self.subTest("token not found"): org = G(Owner) - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) params = { "version": "v4", @@ -330,7 +330,7 @@ def test_determine_repo_upload(self): @patch.object(requests, "get") def test_determine_repo_upload_tokenless(self, mock_get): org = G(Owner, username="codecov", service="github") - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) expected_response = { "id": 732059764, "finishTime": f"{datetime.now()}", @@ -503,7 +503,7 @@ def test_determine_upload_commit_to_use( service="bitbucket", oauth_token=encryptor.encode("hahahahaha").decode(), ) - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) upload_params = { "service": "bitbucket", "commit": "3be5c52bd748c508a7e96993c02cf3518c816e84", @@ -519,7 +519,7 @@ def test_determine_upload_commit_to_use( service="github", oauth_token=encryptor.encode("hahahahaha").decode(), ) - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -532,7 +532,7 @@ def test_determine_upload_commit_to_use( with self.subTest("just no bot available"): org = G(Owner, service="github", oauth_token=None) - repo = G(Repository, author=org, private=True, activated=True) + repo = G(Repository, author=org, private=True) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -549,7 +549,7 @@ def test_determine_upload_commit_to_use( service="github", oauth_token=encryptor.encode("hahahahaha").decode(), ) - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -564,7 +564,7 @@ def test_determine_upload_commit_to_use( with self.subTest("use repo bot token when available"): bot = OwnerFactory() org = G(Owner, service="github") - repo = G(Repository, author=org, bot=bot, activated=True) + repo = G(Repository, author=org, bot=bot) upload_params = { "service": "github", @@ -597,7 +597,7 @@ def test_determine_upload_commit_to_use( with self.subTest("HTTP error"): org = G(Owner, service="github") - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) upload_params = { "service": "github", "commit": "3084886b7ff869dcf327ad1d28a8b7d34adc7584", @@ -610,7 +610,7 @@ def test_determine_upload_commit_to_use( def test_insert_commit(self): org = G(Owner) - repo = G(Repository, author=org, activated=True) + repo = G(Repository, author=org) with self.subTest("newly created"): insert_commit( @@ -743,7 +743,7 @@ def test_store_report_in_redis(self): def test_validate_upload_repository_moved(self): redis = MockRedis() owner = G(Owner, plan="users-free") - repo = G(Repository, author=owner, name="", activated=True) + repo = G(Repository, author=owner, name="") commit = G(Commit) with self.assertRaises(ValidationError) as err: @@ -757,7 +757,7 @@ def test_validate_upload_repository_moved(self): def test_validate_upload_empty_totals(self): redis = MockRedis() owner = G(Owner, plan="5m") - repo = G(Repository, author=owner, activated=True) + repo = G(Repository, author=owner) commit = G(Commit, totals=None, repository=repo) validate_upload({"commit": commit.commitid}, repo, redis) @@ -769,7 +769,7 @@ def test_validate_upload_empty_totals(self): def test_validate_upload_too_many_uploads_for_commit(self): redis = MockRedis() owner = G(Owner, plan="users-free") - repo = G(Repository, author=owner, activated=True) + repo = G(Repository, author=owner) commit = G(Commit, totals={"s": 151}, repository=repo) report = CommitReportFactory.create(commit=commit) for i in range(151): @@ -782,7 +782,7 @@ def test_validate_upload_too_many_uploads_for_commit(self): def test_validate_upload_repository_blacklisted(self): redis = MockRedis(blacklisted=True) owner = G(Owner, plan="users-free") - repo = G(Repository, author=owner, activated=True) + repo = G(Repository, author=owner) commit = G(Commit) with self.assertRaises(ValidationError) as err: @@ -795,7 +795,7 @@ def test_validate_upload_repository_blacklisted(self): def test_validate_upload_per_repo_billing_invalid(self): redis = MockRedis() owner = G(Owner, plan="1m") - G(Repository, author=owner, private=True, activated=False, active=True) + G(Repository, author=owner, private=True, activated=True, active=True) repo = G(Repository, author=owner, private=True, activated=False, active=False) commit = G(Commit) @@ -876,7 +876,7 @@ def test_validate_upload_valid_upload_repo_activated(self): @freeze_time("2023-01-01T00:00:00") @patch("services.task.TaskService.upload") def test_dispatch_upload_task(self, upload): - repo = G(Repository, activated=True) + repo = G(Repository) task_arguments = { "commit": "commit123", "version": "v4", diff --git a/upload/tests/views/test_commits.py b/upload/tests/views/test_commits.py index 9e4bd7c00c..5b6084c6f4 100644 --- a/upload/tests/views/test_commits.py +++ b/upload/tests/views/test_commits.py @@ -42,6 +42,7 @@ def test_get_repo_not_found(db): upload_views.get_repo() assert exp.match("Repository not found") + def test_deactivated_repo(db): repo = RepositoryFactory( name="the_repo", diff --git a/upload/tests/views/test_reports.py b/upload/tests/views/test_reports.py index 0927e29125..84dce1a153 100644 --- a/upload/tests/views/test_reports.py +++ b/upload/tests/views/test_reports.py @@ -39,6 +39,7 @@ def test_reports_get_not_allowed(client, mocker, db): res = client.get(url, **headers) assert res.status_code == 405 + def test_deactivated_repo(db): repo = RepositoryFactory( name="the_repo", @@ -59,9 +60,7 @@ def test_deactivated_repo(db): args=["github", repo_slug, commit.commitid], ) response = client.post( - url, - data={"code": "code1"}, - headers={"User-Agent": "codecov-cli/0.4.7"} + url, data={"code": "code1"}, headers={"User-Agent": "codecov-cli/0.4.7"} ) response_json = response.json() assert response.status_code == 400 diff --git a/upload/views/commits.py b/upload/views/commits.py index 268c94a22e..71ace788ed 100644 --- a/upload/views/commits.py +++ b/upload/views/commits.py @@ -14,7 +14,10 @@ repo_auth_custom_exception_handler, ) from core.models import Commit -from upload.helpers import generate_upload_prometheus_metrics_labels, validate_activated_repo +from upload.helpers import ( + generate_upload_prometheus_metrics_labels, + validate_activated_repo, +) from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import CommitSerializer from upload.views.base import GetterMixin diff --git a/upload/views/reports.py b/upload/views/reports.py index 2e1d90fa0f..9ee4cd6d62 100644 --- a/upload/views/reports.py +++ b/upload/views/reports.py @@ -16,7 +16,10 @@ ) from reports.models import CommitReport, ReportResults from services.task import TaskService -from upload.helpers import generate_upload_prometheus_metrics_labels, validate_activated_repo +from upload.helpers import ( + generate_upload_prometheus_metrics_labels, + validate_activated_repo, +) from upload.metrics import API_UPLOAD_COUNTER from upload.serializers import CommitReportSerializer, ReportResultsSerializer from upload.views.base import GetterMixin From 6c1307b7c37b99239ba82b54ee2818091be70abd Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 30 Oct 2024 14:31:19 -0700 Subject: [PATCH 3/4] adjust changes --- api/public/v2/tests/test_api_repo_viewset.py | 4 +-- upload/helpers.py | 29 ++++++++++++++------ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/api/public/v2/tests/test_api_repo_viewset.py b/api/public/v2/tests/test_api_repo_viewset.py index 790078d18b..10579207c8 100644 --- a/api/public/v2/tests/test_api_repo_viewset.py +++ b/api/public/v2/tests/test_api_repo_viewset.py @@ -62,7 +62,7 @@ def test_list(self): "language": self.repo.language, "branch": "main", "active": False, - "activated": True, + "activated": False, "totals": { "branches": 0, "complexity": 0.0, @@ -115,7 +115,7 @@ def test_retrieve(self, get_repo_permissions): "language": self.repo.language, "branch": "main", "active": False, - "activated": True, + "activated": False, "totals": { "branches": 0, "complexity": 0.0, diff --git a/upload/helpers.py b/upload/helpers.py index 7384a1e325..edf03ab996 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -790,7 +790,7 @@ def get_version_from_headers(headers): return "unknown-user-agent" -def generate_upload_sentry_metrics_tags( +def generate_upload_prometheus_metrics_labels( action, request, is_shelter_request, @@ -798,6 +798,7 @@ def generate_upload_sentry_metrics_tags( repository: Optional[Repository] = None, position: Optional[str] = None, upload_version: Optional[str] = None, + include_empty_labels: bool = True, ): metrics_tags = dict( agent=get_agent_from_headers(request.headers), @@ -806,13 +807,23 @@ def generate_upload_sentry_metrics_tags( endpoint=endpoint, is_using_shelter="yes" if is_shelter_request else "no", ) + + repo_visibility = None if repository: - metrics_tags["repo_visibility"] = ( - "private" if repository.private is True else "public" - ) - if position: - metrics_tags["position"] = position - if upload_version: - metrics_tags["upload_version"] = upload_version + repo_visibility = "private" if repository.private else "public" + + optional_fields = { + "repo_visibility": repo_visibility, + "position": position, + "upload_version": upload_version, + } + + metrics_tags.update( + { + field: value + for field, value in optional_fields.items() + if value or include_empty_labels + } + ) - return metrics_tags + return metrics_tags \ No newline at end of file From c1e2dd8d42647fbf42fbc9b73f8baeea5d79b5b4 Mon Sep 17 00:00:00 2001 From: Adrian Date: Wed, 30 Oct 2024 15:26:25 -0700 Subject: [PATCH 4/4] lint --- upload/helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upload/helpers.py b/upload/helpers.py index edf03ab996..443c1b0f35 100644 --- a/upload/helpers.py +++ b/upload/helpers.py @@ -826,4 +826,4 @@ def generate_upload_prometheus_metrics_labels( } ) - return metrics_tags \ No newline at end of file + return metrics_tags