Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use git diff instead of GitHub's API to detect if manifest fields changed during validation #7599

Merged
merged 2 commits into from
Sep 16, 2020
Merged

Use git diff instead of GitHub's API to detect if manifest fields changed during validation #7599

merged 2 commits into from
Sep 16, 2020

Conversation

nmuesch
Copy link
Collaborator

@nmuesch nmuesch commented Sep 16, 2020

What does this PR do?

Updates the ddev validate manifest command to:
* remove the -i option
* remove the repo_url option

and switch the method used to detect if a manifest file changed to use a local git diff instead of the github api.

Motivation

Using local git is much faster (and less error prone) than reaching out to the github API for each integration's manifest content. Locally the runtime of the command went from ~45 seconds down to ~3 seconds.

Additionally this lets us work with private integration repos since the detection is all done locally.

This comes with the drawback of no longer supporting the fix flag for this specific part of the validation.

Additional Notes

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

@nmuesch nmuesch requested a review from a team as a code owner September 16, 2020 17:40
@ghost ghost added the dev/tooling label Sep 16, 2020
@nmuesch nmuesch requested a review from a team as a code owner September 16, 2020 17:44
@codecov
Copy link

codecov bot commented Sep 16, 2020 •
edited
Loading

Codecov Report

Merging #7599 into master will decrease coverage by 7.28%.
The diff coverage is 25.00%.

Impacted Files Coverage Δ
...tadog_checks_dev/datadog_checks/dev/tooling/git.py 73.07% <25.00%> (-1.93%) ⬇️
lighttpd/datadog_checks/lighttpd/__init__.py
...adog_checks/powerdns_recursor/powerdns_recursor.py
fluentd/tests/test_unit.py
hazelcast/datadog_checks/hazelcast/__init__.py
gunicorn/tests/test_unit.py
nginx/tests/test_unit.py
airflow/datadog_checks/airflow/airflow.py
pgbouncer/datadog_checks/pgbouncer/__init__.py
aspdotnet/datadog_checks/aspdotnet/aspdotnet.py
... and 770 more

@ofek ofek changed the title Use git diff instead of git api to detect if manifest fields changed during validation Use git diff instead of GitHub's API to detect if manifest fields changed during validation Sep 16, 2020
ofek
ofek requested changes Sep 16, 2020
View reviewed changes
Copy link
Contributor

@ofek ofek left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...

ofek
ofek approved these changes Sep 16, 2020
View reviewed changes
Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

apigirl
apigirl approved these changes Sep 16, 2020
View reviewed changes
Copy link
Contributor

@apigirl apigirl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docs look good

@nmuesch nmuesch merged commit 6369229 into master Sep 16, 2020
@nmuesch nmuesch deleted the nick/mainfest_i branch September 16, 2020 21:47
github-actions bot pushed a commit that referenced this pull request Sep 16, 2020
@nmuesch
Use git diff instead of GitHub's API to detect if manifest fields c…
5e3c7c0 
…hanged during validation (#7599)

* Use git diff instead of git api to detect if manifest fields changed

* remove references to -i 6369229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apigirl apigirl apigirl approved these changes

@ofek ofek ofek approved these changes

Assignees
No one assigned
Labels
dev/testing dev/tooling documentation integration/datadog_checks_dev
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nmuesch @ofek @apigirl