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

Improved templating audits #238

Merged
merged 42 commits into from
Jan 24, 2025
Merged

Improved templating audits #238

merged 42 commits into from
Jan 24, 2025

Conversation

kenodegard
Copy link
Contributor

@kenodegard kenodegard commented Dec 17, 2024

Description

Depends on #243

Whoa this became kinda complicated...no wonder I originally skipped this stuff.

I was reminded (in conda/conda-libmamba-solver#588 (comment)) that I had chosen not to over engineered the templating engine but with the templates becoming more complex (and others besides myself monitoring the templating jobs) we might as well include all the bells and whistles to help ourselves when debugging.

Old Audit

old audit

New Audit

new audit

This change introduces a few key improvements:

  1. we now watch for context usage so we can warn the user about missing variables, the context has 4 possible states:

    State Description
    context a variable that has been successfully used in the template
    optional a variable that has not been defined but has a fallback value in the template
    missing a variable that has not been defined and has no fallback value
    unused a variable that is not used in the template
  2. if the templating produces errors we will leave the audit open by default, this will make the errors more noticeable

  3. the template stubs table also includes counts of how many times the stubs are referenced

@kenodegard kenodegard requested a review from a team as a code owner December 17, 2024 06:16
@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Dec 17, 2024
dbast
dbast previously approved these changes Dec 17, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pre-commit/ruff changes

template-files/action.py Outdated Show resolved Hide resolved
template-files/action.py Outdated Show resolved Hide resolved
template-files/action.py Outdated Show resolved Hide resolved
@travishathaway
Copy link
Contributor

@kenodegard,

Maybe a little outside the scope of this pull request, but why isn't the code in this repository being tested? 😅

If you could, maybe now would be a good time to introduce it if only for this code that is changing in this pull request.

@travishathaway
Copy link
Contributor

I created a separate issue for adding a testing actions:

@kenodegard
Copy link
Contributor Author

Maybe a little outside the scope of this pull request, but why isn't the code in this repository being tested? 😅

GitHub Actions/Workflows are not exactly easy to test in many cases since they're always running in "production", this GHA is probably an easier one to test since the action defers to a standalone script, but yes it would be good to figure something out testing wise

@kenodegard kenodegard force-pushed the template-filess-audit-context branch from eb76256 to 8eaa05a Compare January 10, 2025 03:10
@kenodegard kenodegard force-pushed the template-filess-audit-context branch 8 times, most recently from bc0e4a9 to 55f3449 Compare January 10, 2025 05:07
@kenodegard kenodegard force-pushed the template-filess-audit-context branch from 55f3449 to c595976 Compare January 10, 2025 05:12

This comment was marked as outdated.

@conda conda deleted a comment from github-actions bot Jan 10, 2025
Copy link

github-actions bot commented Jan 10, 2025

Template Success

Templating Audit
  • 🔄 Fetching files from ./templates
    • file1.txt.github_cache/template-files/success/file1.txt
      • ✅ (used) stub1.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
    • ⚠️ file2.txt.github_cache/template-files/success/file2.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
      • 📚 (context) variable=value
      • ➕ (optional) optional=
Stub State Count
stub1.txt ✅ (used) 1
stub2.txt ✅ (used) 2
stub3.txt ❌ (missing) 2

Template Error

Templating Audit
  • 🔄 Fetching files from ./templates
    • file1.txt.github_cache/template-files/error/file1.txt
      • ✅ (used) stub1.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
    • ❌ Context missing file2.txt.github_cache/template-files/error/file2.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
      • ❌ (missing) variable=
      • ➕ (optional) optional=
Stub State Count
stub1.txt ✅ (used) 1
stub2.txt ✅ (used) 2
stub3.txt ❌ (missing) 2

Got 1 error(s)

@kenodegard kenodegard force-pushed the template-filess-audit-context branch from a72247b to 73ae56d Compare January 10, 2025 05:29
@kenodegard kenodegard marked this pull request as ready for review January 10, 2025 15:56
@kenodegard kenodegard requested a review from jezdez January 15, 2025 23:07
@conda-bot
Copy link
Contributor

conda-bot commented Jan 22, 2025

Template Success

Warning

This is what the audit looks like when the templating has no errors.

Templating Audit
  • 🔄 Fetching files from ./templates
    • file1.txt.github_cache/template-files/success/file1.txt
      • ✅ (used) stub1.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
    • ⚠️ file2.txt.github_cache/template-files/success/file2.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
      • 📚 (context) variable=value
      • ➕ (optional) optional=
Stub State Count
stub1.txt ✅ (used) 1
stub2.txt ✅ (used) 2
stub3.txt ❌ (missing) 2
### Template Error > [!WARNING] > This is what the audit looks like when templating results in errors.
Templating Audit
  • 🔄 Fetching files from ./templates
    • file1.txt.github_cache/template-files/error/file1.txt
      • ✅ (used) stub1.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
    • ❌ Context missing file2.txt.github_cache/template-files/error/file2.txt
      • ✅ (used) stub2.txt
      • ❌ (missing) stub3.txt
      • ❌ (missing) variable=
      • ➕ (optional) optional=
Stub State Count
stub1.txt ✅ (used) 1
stub2.txt ✅ (used) 2
stub3.txt ❌ (missing) 2

Got 1 error(s)

@jezdez jezdez merged commit 7873f9d into main Jan 24, 2025
7 checks passed
@jezdez jezdez deleted the template-filess-audit-context branch January 24, 2025 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Status: 🏁 Done
Development

Successfully merging this pull request may close these issues.

5 participants