[LOGSC-1681] Add a check to validate that logs assets are owned by logs team #17185

txominpelu · 2024-03-14T14:54:33Z

What does this PR do?

After the migration of logs assets to the APW repos, we've had to update the github CODEOWNER files in all repos to make sure that logs assets for all integrations are owned by logs. With this PR we are adding a check to make sure that this condition is always met.

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
Changelog entries must be created for modifications to shipped code
Add the qa/skip-qa label if the PR doesn't need to be tested during QA.

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py

github-actions · 2024-03-15T14:38:17Z

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

github-actions · 2024-03-15T14:45:00Z

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

codecov · 2024-03-15T15:00:56Z

Codecov Report

Attention: Patch coverage is 15.84158% with 85 lines in your changes are missing coverage. Please review.

Project coverage is 90.45%. Comparing base (4145d3e) to head (3706324).

Additional details and impacted files

Flag	Coverage Δ
cassandra	`?`
confluent_platform	`?`
datadog_checks_dev	`77.41% <15.84%> (+1.44%)`	⬆️
hudi	`?`
kafka	`?`
solr	`?`
tomcat	`?`
weblogic	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

…gs team

iliakur

Thanks!

Just a couple small things.

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py

datadog_checks_dev/datadog_checks/dev/tooling/codeowners.py

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py

Co-authored-by: Ilia Kurenkov <ilia.kurenkov@datadoghq.com>

iliakur · 2024-03-18T14:28:54Z

@txominpelu could you add a changelog entry and run ddev test -fs datadog_checks_dev.

txominpelu · 2024-03-18T15:54:09Z

@txominpelu could you add a changelog entry and run ddev test -fs datadog_checks_dev.

Sure, does this look good to you ?2fad0b3

I've also run the command that you've suggested but it's only changing files that I haven't touched.

github-actions · 2024-03-18T16:12:20Z

The validations job has failed; please review the Files changed tab for possible suggestions to resolve.

…ogs_assets

This reverts commit a6a979e.

github-actions · 2024-03-19T07:35:24Z

Test Results

4 files 4 suites 3m 41s ⏱️
391 tests 391 ✅ 0 💤 0 ❌
882 runs 849 ✅ 33 💤 0 ❌

Results for commit cc7deb8.

♻️ This comment has been updated with latest results.

FlorentClarret · 2024-03-21T10:14:17Z

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py

+    codeowner_map = create_codeowners_map()
+


Suggested change

codeowner_map = create_codeowners_map()

That's initilialized in the if statement right after

FlorentClarret · 2024-03-21T10:15:11Z

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py

+    if failed_integrations:
+        for integration in failed_integrations:
+            echo_failure(f"/{integration}/assets/logs/ is not owned by {LOGS_TEAM}")
+        abort()


Suggested change

abort()

has_failed = True

I would drop it so we can go through the rest of the validation after :)

FlorentClarret · 2024-03-21T10:15:49Z

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py

+        echo_success("All integrations have valid logs codeowners.")
+
+    codeowner_map = create_codeowners_map()
+
    has_failed = False


and thus move this to the top 🙂

Do you mean to the bottom ? Like, this commit: cc7deb8

I meant has_failed = False, so it works if you put your code at the bottom 😄

aaaah, haha. Makes sense, I did it even if I had not fully understood your comment :P

iliakur

Just one small request, otherwise lgtm!

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py

…ogs_assets

datadog-agent-integrations-bot bot added dev/tooling dev_package labels Mar 14, 2024

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch from e4e36ab to 8362890 Compare March 15, 2024 14:28

txominpelu commented Mar 15, 2024

View reviewed changes

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py Outdated Show resolved Hide resolved

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch from 8362890 to 35492d6 Compare March 15, 2024 14:36

datadog-agent-integrations-bot bot added release qa/skip-qa Automatically skip this PR for the next QA labels Mar 15, 2024

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch from 35492d6 to 678af31 Compare March 15, 2024 14:43

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch 2 times, most recently from 6c7baf6 to d5e5642 Compare March 15, 2024 14:55

txominpelu changed the base branch from master to florentclarret/dcd/codeowners March 15, 2024 14:55

FlorentClarret force-pushed the florentclarret/dcd/codeowners branch from 0c8860e to 9245dbc Compare March 15, 2024 14:55

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch from 75281f1 to d5e5642 Compare March 15, 2024 15:05

FlorentClarret force-pushed the florentclarret/dcd/codeowners branch from 9245dbc to ab8f0bf Compare March 15, 2024 15:48

Base automatically changed from florentclarret/dcd/codeowners to master March 15, 2024 15:57

[LOGSC-1681] Add a check to validate that logs assets are owned by lo…

ca09e9d

…gs team

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch from d5e5642 to ca09e9d Compare March 15, 2024 16:17

txominpelu marked this pull request as ready for review March 15, 2024 16:18

txominpelu requested a review from a team as a code owner March 15, 2024 16:18

datadog-agent-integrations-bot bot added the team/agent-integrations label Mar 15, 2024

iliakur reviewed Mar 15, 2024

View reviewed changes

txominpelu and others added 2 commits March 18, 2024 13:56

Update datadog_checks_dev/datadog_checks/dev/tooling/codeowners.py

56d02c7

Co-authored-by: Ilia Kurenkov <ilia.kurenkov@datadoghq.com>

Set the license reference at the top of the file

a4b3f32

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch from 559972c to a4b3f32 Compare March 18, 2024 13:00

Return failed integrations in validate_logs_assets

456f7c7

The check fails if logs assets don't have the right owners

61224bd

Add changelog entry

76eded6

txominpelu added 2 commits March 18, 2024 17:17

Restore wrongly removed code

1d9750f

Lint python files

a6a979e

urseberry previously approved these changes Mar 18, 2024

View reviewed changes

Change code after linter check

5e1bf62

txominpelu dismissed urseberry’s stale review via 5e1bf62 March 18, 2024 22:01

txominpelu force-pushed the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch from 027c445 to 5e1bf62 Compare March 18, 2024 22:01

txominpelu and others added 4 commits March 18, 2024 23:02

Merge branch 'master' into inigo.mediavilla/enforce_logs_owners_for_l…

fa93c59

…ogs_assets

Revert "Lint python files"

cc9ad57

This reverts commit a6a979e.

lint

5b77fb1

changelog

44e78d6

FlorentClarret removed the release label Mar 19, 2024

FlorentClarret and others added 2 commits March 19, 2024 08:41

changelog

de9d9a9

Drop run.py file used for local testing

c428222

txominpelu requested review from iliakur and urseberry and removed request for urseberry March 19, 2024 09:04

FlorentClarret reviewed Mar 21, 2024

View reviewed changes

Report all errors instead of aborting after first failure

cc7deb8

iliakur reviewed Mar 21, 2024

View reviewed changes

datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/codeowners.py Outdated Show resolved Hide resolved

txominpelu added 2 commits March 21, 2024 12:50

Moving hasFailed assignment / initialization

280e802

Merge branch 'master' into inigo.mediavilla/enforce_logs_owners_for_l…

3706324

…ogs_assets

iliakur approved these changes Mar 21, 2024

View reviewed changes

FlorentClarret enabled auto-merge (squash) March 21, 2024 11:56

FlorentClarret merged commit 1e82fcb into master Mar 21, 2024
42 checks passed

FlorentClarret deleted the inigo.mediavilla/enforce_logs_owners_for_logs_assets branch March 21, 2024 12:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LOGSC-1681] Add a check to validate that logs assets are owned by logs team #17185

[LOGSC-1681] Add a check to validate that logs assets are owned by logs team #17185

txominpelu commented Mar 14, 2024 •

edited

Loading

github-actions bot commented Mar 15, 2024

github-actions bot commented Mar 15, 2024

codecov bot commented Mar 15, 2024 •

edited

Loading

iliakur left a comment

iliakur commented Mar 18, 2024

txominpelu commented Mar 18, 2024 •

edited

Loading

github-actions bot commented Mar 18, 2024

github-actions bot commented Mar 19, 2024 •

edited

Loading

FlorentClarret Mar 21, 2024

FlorentClarret Mar 21, 2024 •

edited

Loading

FlorentClarret Mar 21, 2024

txominpelu Mar 21, 2024

FlorentClarret Mar 21, 2024

txominpelu Mar 21, 2024

iliakur left a comment

[LOGSC-1681] Add a check to validate that logs assets are owned by logs team #17185

[LOGSC-1681] Add a check to validate that logs assets are owned by logs team #17185

Conversation

txominpelu commented Mar 14, 2024 • edited Loading

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

github-actions bot commented Mar 15, 2024

github-actions bot commented Mar 15, 2024

codecov bot commented Mar 15, 2024 • edited Loading

Codecov Report

iliakur left a comment

Choose a reason for hiding this comment

iliakur commented Mar 18, 2024

txominpelu commented Mar 18, 2024 • edited Loading

github-actions bot commented Mar 18, 2024

github-actions bot commented Mar 19, 2024 • edited Loading

Test Results

FlorentClarret Mar 21, 2024

Choose a reason for hiding this comment

FlorentClarret Mar 21, 2024 • edited Loading

Choose a reason for hiding this comment

FlorentClarret Mar 21, 2024

Choose a reason for hiding this comment

txominpelu Mar 21, 2024

Choose a reason for hiding this comment

FlorentClarret Mar 21, 2024

Choose a reason for hiding this comment

txominpelu Mar 21, 2024

Choose a reason for hiding this comment

iliakur left a comment

Choose a reason for hiding this comment

txominpelu commented Mar 14, 2024 •

edited

Loading

codecov bot commented Mar 15, 2024 •

edited

Loading

txominpelu commented Mar 18, 2024 •

edited

Loading

github-actions bot commented Mar 19, 2024 •

edited

Loading

FlorentClarret Mar 21, 2024 •

edited

Loading