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.

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

feat: tool to print out the diff between netplan and networkv2 schema #5200

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

catmsred
Copy link
Collaborator

@catmsred catmsred commented Apr 22, 2024

Proposed Commit Message

feat: tool to print out the diff between netplan and networkv2 schema

Since netplan contains many keys that do not exist in the cloud-init
networkv2 schema.  This change adds a tools script that print the
difference between netplan's examples (since they do not have a
schema as such) and the cloud-init networkv2 schema.

Checklist

  • My code follows the process laid out in the documentation
  • I have updated or added any unit tests accordingly -- no unit tests for tools
  • I have updated or added any documentation accordingly -- see docstring which will print as --help

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

@catmsred catmsred added the wip Work in progress, do not land label Apr 22, 2024
@catmsred
Copy link
Collaborator Author

Still TODO: current code assumes (a) the user is running from the cloudinit top level directory and (b) has netplan repo cloned into ../netplan, neither of which are reasonable assumptions and should be fixed

@catmsred catmsred force-pushed the feature/netplan_schema_sync branch 5 times, most recently from d8f14a3 to 5e21354 Compare April 25, 2024 11:26
@catmsred catmsred force-pushed the feature/netplan_schema_sync branch 2 times, most recently from d62bd82 to 6ff01b3 Compare April 25, 2024 16:52
@catmsred catmsred removed the wip Work in progress, do not land label Apr 25, 2024
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Script overall looks good to me.

Can we not depend on click though? For such a small tool I don't think there's really any value add for requiring another dependency...though I'll be completely honest in saying that I never think there's a value add with click 😉

Using argparse instead would look something like:

diff --git a/tools/netplan_schema_check.py b/tools/netplan_schema_check.py
index 2dddff358..d85a7985a 100644
--- a/tools/netplan_schema_check.py
+++ b/tools/netplan_schema_check.py
@@ -1,17 +1,10 @@
-import click
+import argparse
 import os
 import yaml
 
 from jsonschema import Draft4Validator
 
 
-@click.command()
-@click.option(
-    "--schema_file", required=True, help="path to network v2 schema file"
-)
-@click.option(
-    "--netplan_examples", required=True, help="path to netplan examples"
-)
 def validate_netplan_against_networkv2(schema_file, netplan_examples):
     """
     This script validates netplan example files against the cloud-init
@@ -65,6 +58,23 @@ def validate_netplan_against_networkv2(schema_file, netplan_examples):
     print(yaml.dump(error_obj))
 
 
-# pylint: disable=no-value-for-parameter
+def parse_args() -> argparse.Namespace:
+    parser = argparse.ArgumentParser(
+        description="Validate netplan examples against networkv2 schema"
+    )
+    parser.add_argument(
+        "--schema-file",
+        required=True,
+        help="path to network v2 schema file",
+    )
+    parser.add_argument(
+        "--netplan-examples",
+        required=True,
+        help="path to netplan examples",
+    )
+    return parser.parse_args()
+
+
 if __name__ == "__main__":
-    validate_netplan_against_networkv2()
+    args = parse_args()
+    validate_netplan_against_networkv2(args.schema_file, args.netplan_examples)

@TheRealFalcon TheRealFalcon self-assigned this Apr 25, 2024
@catmsred catmsred force-pushed the feature/netplan_schema_sync branch from 6ff01b3 to 5f3e623 Compare April 26, 2024 13:10
@catmsred
Copy link
Collaborator Author

@TheRealFalcon I have no real attachment to click; updated with argparse

Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

LGTM!

@TheRealFalcon TheRealFalcon merged commit 22f4ff8 into canonical:main Apr 26, 2024
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants