chore(ci_visibility): create client for test visibility backend API #10752
Conversation
|
|
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| else: | ||
| return EVPProxyTestVisibilityClient( | ||
| itr_skipping_level, | ||
| git_data, | ||
| self.default_configurations, | ||
| agent_url, | ||
| dd_service, | ||
| dd_env, | ||
| client_timeout, | ||
| ) |
There was a problem hiding this comment.
⚪ Code Quality Violation
else is not necessary since the if clause has a return (...read more)
If the code in the if branch returns a value, do not have the else branch present.
|
|
||
|
|
||
| def _get_mock_connection(body): | ||
| class _mock_http_response: |
There was a problem hiding this comment.
⚪ Code Quality Violation
class names should be CamelCase (...read more)
Class names should be CapWords and not camelCase or snake_case.
Learn More
| except: # noqa: E722 | ||
| log.debug("Failed to parse coverage data for file %s", covered_file) | ||
| parse_errors += 1 | ||
| continue |
There was a problem hiding this comment.
🟠 Code Quality Violation
no bare except (...read more)
Avoid bare except. Try to always use specialized exception names in except blocks.
| else: | ||
| return EVPProxyTestVisibilityClient( | ||
| itr_skipping_level, | ||
| git_data, | ||
| self.default_configurations, | ||
| agent_url, | ||
| dd_service, | ||
| dd_env, | ||
| client_timeout, | ||
| ) |
There was a problem hiding this comment.
⚪ Code Quality Violation
else is not necessary since the if clause has a return (...read more)
If the code in the if branch returns a value, do not have the else branch present.
|
|
||
|
|
||
| def _get_mock_connection(body): | ||
| class _mock_http_response: |
There was a problem hiding this comment.
⚪ Code Quality Violation
class names should be CamelCase (...read more)
Class names should be CapWords and not camelCase or snake_case.
Learn More
| except: # noqa: E722 | ||
| log.debug("Failed to parse coverage data for file %s", covered_file) | ||
| parse_errors += 1 | ||
| continue |
There was a problem hiding this comment.
🟠 Code Quality Violation
no bare except (...read more)
Avoid bare except. Try to always use specialized exception names in except blocks.
Datadog ReportBranch report: ✅ 0 Failed, 592 Passed, 694 Skipped, 20m 3.81s Total duration (16m 59.4s time saved) |
BenchmarksBenchmark execution time: 2024-09-23 17:09:48 Comparing candidate commit 6df23bd in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 353 metrics, 47 unstable metrics. |
| parse_errors = 0 | ||
| for covered_file, covered_lines_bytes in covered_files_data.items(): | ||
| try: | ||
| covered_lines = CoverageLines.from_bytearray(bytearray(b64decode(covered_lines_bytes))) |
There was a problem hiding this comment.
I was going to say that we already have a from_b64_string method in CoverageLines, but it turns out it's missing the b64decode() there (which I think is wrong).
There was a problem hiding this comment.
😓 that's because I started adding that method and the decided it wasn't worth adding as a new method to the API (but then neglected to remove it).
| try: | ||
| suite_id = _get_suite_id_from_skippable_suite(skippable_suite) | ||
| suites_to_skip.add(suite_id) | ||
| except Exception: # noqa: E722 |
There was a problem hiding this comment.
Sometimes we use bare except and sometimes we use except Exception, should we standardize on a single one?
There was a problem hiding this comment.
Good point. I think I just lack enough of an opinion to pick one. :D Maybe just except: ?
There was a problem hiding this comment.
Well, the code quality thingy seems to complain about bare except: but not about except Exception: (but then again, it may be updated to complain about except Exception: too at some point, so this is not a strong argument). One potential point in favor of except Exception is that it makes you stop and thing "do I really need to catch every exception?" (but then again, in the tracer we often do want that).
| @@ -68,7 +68,7 @@ def subprocess_run(self, *args): | |||
|
|
|||
| def test_and_emit_get_version(self): | |||
| version = get_version() | |||
| assert type(version) == str | |||
| assert type(version) is str | |||
There was a problem hiding this comment.
⚪ Code Quality Violation
use isintance() instead of type() (...read more)
Using isinstance is faster than type but also consider inheritance, which makes it more accurate.
This creates a new client for the Test Visibility API.
Checklist
Reviewer Checklist