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(eos_cli_config_gen): Add schema for sflow #2056

Merged
merged 5 commits into from
Sep 14, 2022

Conversation

carlbuchmann
Copy link
Member

@carlbuchmann carlbuchmann commented Sep 14, 2022

Add schema for data model

Fix issue with sflow.sample type/auto conversion, and adding sflow.hardware_accelleration key missed in PR #2036

Additionally, now enforcing validation on the eos_cli_config_gen molecule scenario to trigger CI failure.
See commit d3a41fd which triggered CI failure.

Checklist

Contributor Checklist

  • Create schema fragment matching data model described in README.md and README_v4.0.md
    • README.md is most complete with all keys. README_v4.0 includes partial data models after conversion to lists.
    • Schema fragment path is roles/eos_cli_config_gen/schemas/schema_fragments/<data_model_key>.schema.yml.
    • Copy line 1-5 from another schema (comments and outer type:dict).
    • Refer to schema documentation for syntax and/or use YAML Lint plugin from Redhat in VSCode.
    • Use convert_types on value that could be mixed type or misinterpreted like integers and numeric strings.
  • If the data model has been converted from wildcard dicts:
    • Add convert_types: ['dict'] to the schema.
    • Remove convert_dicts from the templates/eos/<>.j2 and templates/documentation/<>.j2 templates.
  • Run molecule converge -s build_schemas_and_docs to update schema and documentation.
  • Test by running molecule converge -s eos_cli_config_gen and verify no errors or changes to generated configs/docs.

Reviewer Checklist

  • Reviewer 1:

    • Verify that data model is fully covered in the described schema. Easiest by looking at the generated documentation.
    • Verify that convert_dicts has been removed from templates as applicable.
    • Verify no changes to configs/docs on any molecule scenario
    • Verify that CI pass
  • Reviewer 2:

    • Verify that data model is fully covered in the described schema. Easiest by looking at the generated documentation.
    • Verify that convert_dicts has been removed from templates as applicable.
    • Verify no changes to configs/docs on any molecule scenario
    • Verify that CI pass

@carlbuchmann carlbuchmann requested a review from a team as a code owner September 14, 2022 00:19
@github-actions github-actions bot added the state: CI Updated CI scenario have been updated in the PR label Sep 14, 2022
@carlbuchmann carlbuchmann changed the title Feat(eos_cli_config_gen): Schema for sflow Feat(eos_cli_config_gen): Add schema for sflow Sep 14, 2022
@carlbuchmann carlbuchmann marked this pull request as draft September 14, 2022 00:25
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

LGTM. Would be good to add this to all the other scenarios as well, since we have eos_cli_config_gen data everywhere, and everything may not be covered in all permutations inside eos_cli_config_gen.

Copy link
Contributor

@Shivani-chourasiya Shivani-chourasiya left a comment

Choose a reason for hiding this comment

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

LGTM!

@carlbuchmann
Copy link
Member Author

LGTM. Would be good to add this to all the other scenarios as well, since we have eos_cli_config_gen data everywhere, and everything may not be covered in all permutations inside eos_cli_config_gen.

Agreed, added to all molecule scenarios in commit: 3baf0b3

@carlbuchmann carlbuchmann merged commit 58d3a1e into aristanetworks:devel Sep 14, 2022
@carlbuchmann carlbuchmann deleted the schema_sflow branch September 14, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants