Skip to content

Commit

Permalink
chore(ci_visibility): don't log exception when codeowners can't be lo…
Browse files Browse the repository at this point in the history
…aded (#10423)

Reverts the `Codeowners.__init__()` behavior introduced in #10094 which
changed the exception being caught by the `CIVisibility` service when
instantiating `Codeowners()`, resulting in undesirable log messages.

`Codeowners.__init__()` raising a `ValueError` exception, caught by the
sole caller (`CIVisibility.__init__()`).

The `of()` behavior is maintained and the caller is updated to check for
`[]` as an indication that no codeowner was found.

There is no release note as #10094 was only backported into `2.12`.

As a bonus, type hints are converted from comments to annotations.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [ ] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
  • Loading branch information
romainkomorndatadog authored Aug 29, 2024
1 parent 16f2960 commit ca182d0
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 14 deletions.
6 changes: 4 additions & 2 deletions ddtrace/internal/ci_visibility/recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,8 +683,10 @@ def set_codeowners_of(cls, location, span=None):
handles = cls._instance._codeowners.of(location)
if handles:
span.set_tag(test.CODEOWNERS, json.dumps(handles))
except KeyError:
log.debug("no matching codeowners for %s", location)
else:
log.debug("no matching codeowners for %s", location)
except: # noqa: E722
log.debug("Error setting codeowners for %s", location, exc_info=True)

@classmethod
def add_session(cls, session: CIVisibilitySession):
Expand Down
29 changes: 17 additions & 12 deletions ddtrace/internal/codeowners.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import os
import re
from typing import List # noqa:F401
from typing import Optional # noqa:F401
from typing import Tuple # noqa:F401
from typing import List
from typing import Optional
from typing import Tuple


def path_to_regex(pattern):
# type: (str) -> re.Pattern
def path_to_regex(pattern: str) -> re.Pattern:
"""
source https://github.com/sbdchd/codeowners/blob/c95e13d384ac09cfa1c23be1a8601987f41968ea/codeowners/__init__.py
Expand Down Expand Up @@ -119,16 +118,20 @@ class Codeowners(object):
".gitlab/CODEOWNERS",
)

def __init__(self, path=None, cwd=None):
# type: (Optional[str], Optional[str]) -> None
def __init__(self, path: Optional[str] = None, cwd: Optional[str] = None):
"""Initialize Codeowners object.
:param path: path to CODEOWNERS file otherwise try to use any from known locations
"""
self.patterns: List[Tuple[re.Pattern, List[str]]] = []
self.path: Optional[str] = None

path = path or self.location(cwd)
if path is not None:
self.path = path # type: str
self.patterns = [] # type: List[Tuple[re.Pattern, List[str]]]

if path is None:
raise ValueError("CODEOWNERS file not found")

self.path = path
self.parse()

def location(self, cwd: Optional[str] = None) -> Optional[str]:
Expand All @@ -140,9 +143,11 @@ def location(self, cwd: Optional[str] = None) -> Optional[str]:
return path
return None

def parse(self):
# type: () -> None
def parse(self) -> None:
"""Parse CODEOWNERS file and store the lines and regexes."""
if self.path is None:
return

with open(self.path) as f:
patterns = []
for line in f.readlines():
Expand Down

0 comments on commit ca182d0

Please sign in to comment.