Add GitHub Actions annotation primitive to ddev's Application#23654
Add GitHub Actions annotation primitive to ddev's Application#23654AAraKKe wants to merge 4 commits into
Conversation
Adds Application.annotate_error/annotate_warning/annotate_display_queue that emit GitHub Actions workflow annotations (::error/::warning) when running on CI and no-op locally. Mirrors the legacy datadog_checks_dev/tooling/commands/console.py helpers so future migrations (and a retrofit of #23651/#23652/#23653) keep CI annotation UX. The display-queue helper takes a list of (level, message) tuples instead of the legacy (func, message) shape, which is idiomatic for ddev's Application-style API.
- Replace ANNOTATION_LEVEL_ERROR/WARNING string constants and the
_GH_ANNOTATION_LEVELS frozenset with a single AnnotationLevel(StrEnum).
Method signatures now type level as AnnotationLevel; the old runtime
"unknown level falls back to error" guard is gone because the enum
enforces membership at construction time.
- Swap annotate_display_queue's pre-populated dict + membership filter
for collections.defaultdict(list). Emission iterates AnnotationLevel
so output order is deterministic (errors before warnings) regardless
of input order — covered by a new test.
- Add a one-line comment near the print() call explaining why it isn't
os.system("echo ...") (the legacy shape was shell-injection prone).
- Tests use AnnotationLevel members directly; new tests for StrEnum
string-identity and the deterministic emission order. 12 tests total.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files🚀 New features to boost your workflow:
|
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: ba95291 | Docs | Datadog PR Page | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38afb36f02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return | ||
| # `print` (not `os.system("echo ...")`): the GH runner parses workflow commands | ||
| # off stdout, and shelling out would corrupt messages containing quotes / `$()`. | ||
| print(f'::{level} file={file},line={line}::{message}') |
There was a problem hiding this comment.
When these helpers are used on GitHub Actions with messages or paths that contain workflow-command control characters, the runner can parse a different command than intended: a literal newline in message terminates the annotation line, %0A in a message is decoded as a newline, and ,/: in file can corrupt the property list. I checked GitHub's workflow-command format (::workflow-command parameter1={data},parameter2={data}::{command value} over stdout) and the official actions/toolkit command implementation, which escapes %, CR, and LF in command data and additionally escapes :, , in properties. Since these helpers accept arbitrary validation/test output, multi-line failures will be truncated or malformed unless file and message are escaped here rather than interpolated raw.
Useful? React with 👍 / 👎.
The previous implementation interpolated message and property values raw into the workflow command, so a literal newline in the message would terminate the annotation early, a `%0A` would be decoded back into a newline, and `,` / `:` in a file path would corrupt the property list. Add `_escape_workflow_data` and `_escape_workflow_property` private helpers that mirror `@actions/core`'s `escapeData` / `escapeProperty`: - Data: % -> %25, \r -> %0D, \n -> %0A - Property: same, plus : -> %3A, , -> %2C Verified against actions/toolkit packages/core/src/command.ts (same operations, same order). `annotate_display_queue` now joins messages with a real `\n` so the escaper produces a single canonical `%0A` sequence instead of double-encoding a literal `%0A`. Tests cover newline / percent / property-separator round-trips plus table-driven coverage of each escaper helper. Existing ASCII-only tests keep passing because the escapers are identity on those inputs.
Validation ReportAll 20 validations passed. Show details
|
1 participant
What does this PR do?
Adds three helpers to ddev's
Applicationso commands can emit GitHub Actions workflow annotations (::error/::warning) that surface inline in PR diffs on CI:Application.annotate_error(file, message, line=1)Application.annotate_warning(file, message, line=1)Application.annotate_display_queue(file, display_queue, line=1)— takes a list of(level, message)tuples and groups same-level messages into a single multi-line annotation using the GitHub%0Anewline escape.All three are no-ops outside CI; the CI check reuses ddev's existing
ddev.utils.ci.running_in_ci()helper (which already recognisesGITHUB_ACTIONS=true|1andCI=true|1). Workflow commands are emitted viaprint()to stdout — that's where the runner parses them, and it avoids the shell-injection footgun of the legacyos.system("echo ...")implementation.Helper gap notes
_emit_github_annotationis intentionally a private method onApplicationrather than a standalone helper. It's only consumed by the two publicannotate_*entry points and reusingApplication's state (and following the existing convention ofdisplay_*living onApplication) keeps the surface compact.(level, message)(ddev style) rather than legacy(func, message). The retrofit PR for the three already-migrated commands will adapt their queues to the new shape.Test plan
ddev --no-interactive test ddevpasses (993 passed; the 3tests/utils/test_github.pyfailures are the documented worktree-vs-cassette noise unrelated to this change — see00-overview.mdKnown issues).ddev test -fs ddevreports no formatter changes.ddev/tests/cli/test_application.pycovers: on-CI emits the workflow command on stdout (parametrized over error/warning); off-CI is silent; customlineis preserved; shell-unsafe characters round-trip cleanly (regression for the legacyos.systemsmell); display-queue groups levels with%0A; unknown levels are skipped; empty queue emits nothing; off-CI display-queue is silent.Motivation
ddev was missing a primitive for the GitHub Actions workflow-command annotations that legacy
datadog_checks_dev/tooling/commands/console.pyexposes asannotate_error/annotate_warning/annotate_display_queue. Three in-flight migration PRs that touched commands using those helpers — #23651 (validate codeowners), #23652 (validate jmx-metrics), and #23653 (validate config) — had to drop the annotation calls, regressing CI annotation UX (errors no longer surface inline in PR diffs on CI).This PR adds the missing primitive to
Applicationso:validate dashboards,validate models,validate service-checks,meta manifest, …) don't have to choose between dropping the feature or porting the helper ad-hoc.No changes to
datadog_checks_dev/. No change to the existingApplicationsurface beyond the three new methods.Review checklist (to be filled by reviewers)
qa/skip-qalabel if the PR doesn't need to be tested during QA.backport/<branch-name>label to the PR and it will automatically open a backport PR once this one is merged