Add validation for readmes #7088

sarah-witt · 2020-07-08T19:27:08Z

What does this PR do?

Adds validation for images in READMEs.

Motivation

Additional Notes

TODO: update developer docs

depends on:

Review checklist (to be filled by reviewers)

Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
PR title must be written as a CHANGELOG entry (see why)
Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
PR must have changelog/ and integration/ labels attached

codecov · 2020-07-08T19:48:11Z

Codecov Report

Merging #7088 into master will decrease coverage by 5.50%.
The diff coverage is n/a.

Impacted Files	Coverage Δ
...ts/tooling/configuration/consumers/test_example.py	`100.00% <0.00%> (ø)`
...cks/dev/tooling/configuration/consumers/example.py	`100.00% <0.00%> (ø)`
kafka/tests/test_check.py
ignite/tests/test_check.py
etcd/datadog_checks/etcd/metrics.py
...ks_fargate/datadog_checks/eks_fargate/__about__.py
openstack/tests/common.py
spark/tests/conftest.py
druid/tests/test_unit.py
sqlserver/tests/utils.py
... and 739 more

ChristineTChen

Looks awesome!! Could we also add validation that the Overview and Setup headers must be there since those are used for tiles tabs?

…into sarah/readme-validation

ChristineTChen · 2020-07-09T20:00:16Z

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

+        if not has_overview and integration not in NON_TILE_INTEGRATIONS:
+            errors = True
+            echo_failure(f"{integration} readme file does not have an overview section")
+
+        if not has_setup and integration not in NON_TILE_INTEGRATIONS:
+            errors = True
+            echo_failure(f"{integration} readme file does not have a setup section")


Suggested change

if not has_overview and integration not in NON_TILE_INTEGRATIONS:

errors = True

echo_failure(f"{integration} readme file does not have an overview section")

if not has_setup and integration not in NON_TILE_INTEGRATIONS:

errors = True

echo_failure(f"{integration} readme file does not have a setup section")

if integration not in NON_TILE_INTEGRATIONS and not has_overview or not has_setup:

errors = True

echo_failure(f"{integration} readme file needs to have an overview and setup section.")

Since these two are same issue (README section headers), I think we could simplify the error. wdyt?

Sure, I could do that

AlexandreYang

Nice 🥇

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

ChristineTChen

LGTM!

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

…e/readmes.py Co-authored-by: Nicholas Muesch <nicholas.muesch@datadoghq.com>

nmuesch

This looks good to me, thanks!

nmuesch · 2020-07-23T20:35:03Z

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

+                            f"https://raw.githubusercontent.com/DataDog/{repo}/master/{integration}/images/<IMAGE_NAME>"
+                        )
+
+                    rel_path = match.groups()[0]


I think need to abort above or put this in an else, otherwise we never see the failure message above if match doesn't exist.

EDIT: sorry you do see the error message above, but it gets drowned out a bit by a stacktrace

Good catch, I updated to break if there's a failure!

…integrations-core into sarah/readme-validation

nmuesch

Nice, thanks for this! I left one note about the formatting of the output but I don't think it needs to block.

nmuesch · 2020-07-24T13:44:12Z

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

+                    match = re.match(IMAGE_REGEX, line)
+                    if not match:
+                        errors = True
+                        echo_failure(f"{integration} readme file does not have a valid image file on line {line_no}")


One thing I noticed is that the other validations print the integration name once when there is an error, then print each error message on a newline with a slight indent, to group errors per integration
Example in the manifest validations - https://github.com/DataDog/integrations-core/blob/master/datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/manifest.py#L289-L301

I like that format! I updated the PR

nmuesch

This looks great, thanks!

* Add utility functions * Add readme validation * Move function * Add to CI * Go back to original comment * Simplify logic * Check for overview and setup * Fix style * Simplify error * Test for exact wording * Fix style * Check that path exists * Update extensions * Remove test for non tile ints * Update datadog_checks_dev/datadog_checks/dev/tooling/commands/validate/readmes.py Co-authored-by: Nicholas Muesch <nicholas.muesch@datadoghq.com> * Break if readme is invalid * Fix style * Fix style * Update to have display queue * Add noqa Co-authored-by: Nicholas Muesch <nicholas.muesch@datadoghq.com> e220790

sarah-witt added 5 commits July 8, 2020 11:13

Add utility functions

fe97679

Add readme validation

a6021c4

Move function

fe2ecff

Add to CI

3318868

Go back to original comment

efa6bb3

github-actions bot added dev/testing dev/tooling integration/datadog_checks_dev labels Jul 8, 2020

Simplify logic

f22b00f

sarah-witt marked this pull request as ready for review July 8, 2020 19:49

sarah-witt requested a review from a team as a code owner July 8, 2020 19:49

sarah-witt added the changelog/Added label Jul 8, 2020

ChristineTChen reviewed Jul 8, 2020

View reviewed changes

sarah-witt added 3 commits July 8, 2020 16:42

Check for overview and setup

a376e88

Merge branch 'master' of https://github.com/DataDog/integrations-core …

3106143

…into sarah/readme-validation

Fix style

c5b3493

sarah-witt requested a review from ChristineTChen July 9, 2020 17:52

ChristineTChen reviewed Jul 9, 2020

View reviewed changes

sarah-witt added 3 commits July 9, 2020 16:09

Simplify error

d19e5fe

Test for exact wording

0f36d80

Fix style

a2ad088

AlexandreYang requested changes Jul 10, 2020

View reviewed changes

sarah-witt added 2 commits July 13, 2020 12:44

Check that path exists

9a2104b

Update extensions

4c16bea

This was referenced Jul 13, 2020

Add setup section to readme #7113

Merged

Add setup section DataDog/integrations-extras#641

Merged

Remove test for non tile ints

890a81c

ChristineTChen previously approved these changes Jul 13, 2020

View reviewed changes

sarah-witt requested a review from AlexandreYang July 15, 2020 13:07

nmuesch reviewed Jul 22, 2020

View reviewed changes

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

Update datadog_checks_dev/datadog_checks/dev/tooling/commands/validat…

774b10c

…e/readmes.py Co-authored-by: Nicholas Muesch <nicholas.muesch@datadoghq.com>

sarah-witt dismissed ChristineTChen’s stale review via 774b10c July 22, 2020 17:54

nmuesch previously approved these changes Jul 22, 2020

View reviewed changes

nmuesch reviewed Jul 23, 2020

View reviewed changes

sarah-witt added 2 commits July 23, 2020 16:47

Break if readme is invalid

cd0dd3c

Merge branch 'sarah/readme-validation' of https://github.com/DataDog/…

4fa05a7

…integrations-core into sarah/readme-validation

sarah-witt dismissed nmuesch’s stale review via 4fa05a7 July 23, 2020 20:49

AlexandreYang previously approved these changes Jul 24, 2020

View reviewed changes

Fix style

954d346

sarah-witt dismissed AlexandreYang’s stale review via 954d346 July 24, 2020 13:24

sarah-witt requested a review from a team as a code owner July 24, 2020 13:24

github-actions bot added documentation integration/apache labels Jul 24, 2020

Fix style

7deaf2c

sarah-witt removed the integration/apache label Jul 24, 2020

sarah-witt requested a review from nmuesch July 24, 2020 13:38

ChristineTChen previously approved these changes Jul 24, 2020

View reviewed changes

nmuesch previously approved these changes Jul 24, 2020

View reviewed changes

Update to have display queue

127556e

sarah-witt dismissed stale reviews from nmuesch and ChristineTChen via 127556e July 24, 2020 14:15

Add noqa

d5f7142

nmuesch approved these changes Jul 24, 2020

View reviewed changes

sarah-witt removed the documentation label Jul 24, 2020

sarah-witt merged commit e220790 into master Jul 24, 2020

sarah-witt deleted the sarah/readme-validation branch July 24, 2020 18:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add validation for readmes #7088

Add validation for readmes #7088

sarah-witt commented Jul 8, 2020

codecov bot commented Jul 8, 2020 •

edited

Loading

ChristineTChen left a comment

ChristineTChen Jul 9, 2020

sarah-witt Jul 9, 2020

AlexandreYang left a comment

ChristineTChen left a comment

nmuesch left a comment

nmuesch Jul 23, 2020

nmuesch Jul 23, 2020

sarah-witt Jul 23, 2020

nmuesch left a comment

nmuesch Jul 24, 2020 •

edited

Loading

sarah-witt Jul 24, 2020

nmuesch left a comment

Add validation for readmes #7088

Add validation for readmes #7088

Conversation

sarah-witt commented Jul 8, 2020

What does this PR do?

Motivation

Additional Notes

Review checklist (to be filled by reviewers)

codecov bot commented Jul 8, 2020 • edited Loading

Codecov Report

ChristineTChen left a comment

Choose a reason for hiding this comment

ChristineTChen Jul 9, 2020

Choose a reason for hiding this comment

sarah-witt Jul 9, 2020

Choose a reason for hiding this comment

AlexandreYang left a comment

Choose a reason for hiding this comment

ChristineTChen left a comment

Choose a reason for hiding this comment

nmuesch left a comment

Choose a reason for hiding this comment

nmuesch Jul 23, 2020

Choose a reason for hiding this comment

nmuesch Jul 23, 2020

Choose a reason for hiding this comment

sarah-witt Jul 23, 2020

Choose a reason for hiding this comment

nmuesch left a comment

Choose a reason for hiding this comment

nmuesch Jul 24, 2020 • edited Loading

Choose a reason for hiding this comment

sarah-witt Jul 24, 2020

Choose a reason for hiding this comment

nmuesch left a comment

Choose a reason for hiding this comment

codecov bot commented Jul 8, 2020 •

edited

Loading

nmuesch Jul 24, 2020 •

edited

Loading