Skip to content

Commit

Permalink
chore(ci_visibility): revert sub-plugin strategy and move v2 function…
Browse files Browse the repository at this point in the history
…s to try-except blocks (#10196)

There are two changes in this commit:

1- remove "sub plugin" strategy introduced in #8999

The sub-plugin strategy was originally implemented to delay the import
of `ddtrace`, however that import is (in the current scenario)
inevitable because the `pyproject.toml` entry-points that tell `pytest`
where to find plugins already import `ddtrace`, and the original
motivation of the change in (#8281 and #4845) was instead addressed by
#9141 .

The motivation to revert this change is that some hooks (eg:
`pytest_load_inital_conftests`) added by the class-based plugins do not
execute when the plugin is loaded in the `pytest_configure()` hook in
the `plugin.py` file itself. All the methods in the plugin classes use
the `@staticmethod` decorator and do not benefit from being in a class.

This change allows the versioned plugin files to implement hooks by
virtue of being imported into the original plugin module.

2- move v2 functions to try-except blocks

Since CI Visibility should never affect `pytest`'s execution (or, at the
very least, should never prevent it from running to completion), the
hooks themselves should never raise exceptions. With this change, some
functions simply wrap the entire body in a try/except block, while some
others define a separate function that is then called in a try/except
block.

A decorator was not used for this purpose because some hooks yield,
while others don't, and logging relevant errors is also a little
simpler.

## 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
- [x] 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
romainkomorn-exdatadog authored Aug 16, 2024
1 parent f481cc2 commit 8db19e6
Show file tree
Hide file tree
Showing 3 changed files with 690 additions and 625 deletions.
Loading

0 comments on commit 8db19e6

Please sign in to comment.