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

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

@@ -7,10 +7,10 @@
 
 import click
 import jsonschema
-import requests
 
 from ....utils import file_exists, read_file, write_file
 from ...constants import get_root
+from ...git import content_changed
 from ...utils import get_metadata_file, parse_version_parts, read_metadata_rows
 from ..console import CONTEXT_SETTINGS, abort, echo_failure, echo_info, echo_success, echo_warning
 
@@ -257,43 +257,19 @@ def is_metric_in_metadata_file(metric, check):
     return False
 
 
-def get_original_manifest_field(check_name, field_to_check, repo_url, manifests_from_master):
-    repo_url = f'{repo_url}/{check_name}/manifest.json'
-    if manifests_from_master.get(check_name):
-        data = manifests_from_master.get(check_name)
-    else:
-        data = requests.get(repo_url.format(check_name))
-        manifests_from_master[check_name] = data
-
-    if data.status_code == 404:
-        # This integration isn't on the provided repo yet, so it's field can change
-        return None
-
-    data.raise_for_status()
-    decoded_master = data.json()
-    return decoded_master.get(field_to_check)
-
-
 @click.command(context_settings=CONTEXT_SETTINGS, short_help='Validate `manifest.json` files')
 @click.option('--fix', is_flag=True, help='Attempt to fix errors')
-@click.option('--include-extras', '-i', is_flag=True, help='Include optional fields, and extra validations')
-@click.option(
-    '--repo_url', '-r', help='The raw github URL to the base of the repository to compare manifest fields against'
-)
 @click.pass_context
-def manifest(ctx, fix, include_extras, repo_url):
+def manifest(ctx, fix):
     """Validate `manifest.json` files."""
     all_guids = {}
-    raw_gh_url = 'https://raw.githubusercontent.com/DataDog/{}/master/'
 
     root = get_root()
     is_extras = ctx.obj['repo_choice'] == 'extras'
     is_marketplace = ctx.obj['repo_choice'] == 'marketplace'
-    formatted_repo_url = repo_url or raw_gh_url.format(os.path.basename(root))
     ok_checks = 0
     failed_checks = 0
     fixed_checks = 0
-    manifests_from_master = {}
 
     echo_info("Validating all manifest.json files...")
     for check_name in sorted(os.listdir(root)):
@@ -498,45 +474,31 @@ def manifest(ctx, fix, include_extras, repo_url):
                 else:
                     display_queue.append((echo_failure, output))
 
-            if include_extras:
-                # Ensure attributes haven't changed
-                for field in FIELDS_NOT_ALLOWED_TO_CHANGE:
-                    original_value = get_original_manifest_field(
-                        check_name, field, formatted_repo_url, manifests_from_master
-                    )
-                    output = f'Attribute `{field}` is not allowed to be modified.'
-                    if original_value and original_value != decoded.get(field):
-                        file_failures += 1
-                        if fix:
-                            decoded[field] = original_value
-                            file_failures -= 1
-                            file_fixed = True
-                            display_queue.append((echo_warning, output))
-                            display_queue.append((echo_success, f'  original {field}: {original_value}'))
-                        else:
-                            output += f" Please use the existing value: {original_value}"
-                            display_queue.append((echo_failure, output))
-                    elif not original_value:
-                        output = f"  No existing field on default branch: {field}"
-                        display_queue.append((echo_warning, output))
+            # is_public
+            correct_is_public = True
+            is_public = decoded.get('is_public')
+            if not isinstance(is_public, bool):
+                file_failures += 1
+                output = '  required boolean: is_public'
 
-                # is_public
-                correct_is_public = True
-                is_public = decoded.get('is_public')
-                if not isinstance(is_public, bool):
-                    file_failures += 1
-                    output = '  required boolean: is_public'
+                if fix:
+                    decoded['is_public'] = correct_is_public
 
-                    if fix:
-                        decoded['is_public'] = correct_is_public
+                    display_queue.append((echo_warning, output))
+                    display_queue.append((echo_success, f'  new `is_public`: {correct_is_public}'))
 
-                        display_queue.append((echo_warning, output))
-                        display_queue.append((echo_success, f'  new `is_public`: {correct_is_public}'))
+                    file_failures -= 1
+                    file_fixed = True
+                else:
+                    display_queue.append((echo_failure, output))
 
-                        file_failures -= 1
-                        file_fixed = True
-                    else:
-                        display_queue.append((echo_failure, output))
+            # Ensure attributes haven't changed
+            manifest_fields_changed = content_changed(file_glob=f"{check_name}/manifest.json")
+            for field in FIELDS_NOT_ALLOWED_TO_CHANGE:
+                if field in manifest_fields_changed:
+                    output = f'Attribute `{field}` is not allowed to be modified. Please revert to original value'
+                    file_failures += 1
+                    display_queue.append((echo_failure, output))
 
             if file_failures > 0:
                 failed_checks += 1

datadog_checks_dev/datadog_checks/dev/tooling/git.py

@@ -34,6 +34,15 @@ def get_current_branch():
         return run_command(command, capture='out').stdout.strip()
 
 
+def content_changed(file_glob="*"):
+    """
+    Return the content changed in the current branch compared to `master`
+    """
+    with chdir(get_root()):
+        output = run_command(f'git diff master -U0 -- "{file_glob}"', capture='out')
+    return output.stdout
+
+
 def files_changed(include_uncommitted=True):
     """
     Return the list of file changed in the current branch compared to `master`