-
Notifications
You must be signed in to change notification settings - Fork 21
Add Support for Templating to GAP #283
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
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Appreciate all the clean-up and improvement in this PR.
Left one comment.
A couple of other thoughts:
- You might want to add another test for create_template with all of the params specified (e.g. verbose).
- You might want to add a test file that tests the subcommand, so that you're getting the equivalent of an end-to-end test but much cheaper (a unit test with mocks).
If either of the above sound tedious but valuable, let me know and I'm happy to pair program or open a PR branched off of this PR to contribute the tests to your feature.
I agree, we need more testing. And, I think we need live testing. My plan is to substitute a current CI test we have with a templated version (modified to fit the test) of the config file. This will be done in a later story. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exemplary work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well done!
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
* Adding support for templating to GAP * Potential fix for code scanning alert no. 159: Unused import Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> * Fixing isort issue created by autofix * Refactoring template creation * Refactoring parser * Adding missing verbose template comment * Reformatting sweep template comment --------- Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
Adds logic to create templates and adds a new subcommand which creates both verbose (with comments) and non-verbose (just options) templates. The default filename for the template is:
genai_perf_config.yaml
. This can be modified by the user by specifying a filename.I have run both of these templates successfully in live testing.
genai-perf create-template:
genai-perf create-template -v