Skip to content

Remove drill performance tests from CI#7417

Merged
erosselli merged 3 commits intomainfrom
erosselli/remove-drill-performance-tests
Feb 19, 2026
Merged

Remove drill performance tests from CI#7417
erosselli merged 3 commits intomainfrom
erosselli/remove-drill-performance-tests

Conversation

@erosselli
Copy link
Copy Markdown
Contributor

@erosselli erosselli commented Feb 18, 2026

Ticket []

Description Of Changes

The drill performance checks are failing on install. After a closer look, these aren't much of a load /performance test anyways and we're already running performance / load tests regularly on deployed envs, so we can safely remove.

Code Changes

Steps to Confirm

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Feb 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Feb 18, 2026 7:39pm
fides-privacy-center Ignored Ignored Feb 18, 2026 7:39pm

Request Review

@erosselli erosselli marked this pull request as ready for review February 18, 2026 19:30
@erosselli erosselli requested review from a team and JadeCara and removed request for a team February 18, 2026 19:30
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 18, 2026

Greptile Summary

Removes the Drill-based performance test job (Performance-Checks) from the CI pipeline. The job was failing on install and provided limited value as a load/performance test — the team already runs more robust performance tests on deployed environments. The PR cleanly removes the CI workflow job, its performance_tests nox session, updates documentation and the Docker test sessions reference doc, and removes the job from the Backend-Checks-Summary gate. The docker_stats, load_tests nox sessions and noxfiles/drill.yml config remain available for optional local use.

Confidence Score: 5/5

  • This PR is safe to merge — it only removes a broken, non-blocking CI job with no impact on application code or other test suites.
  • The changes are purely removal of a CI job that was already configured with continue-on-error: true (non-blocking). No application logic is modified. The workflow, nox session, and documentation changes are consistent and complete. The remaining orphaned helper sessions (docker_stats, load_tests, drill.yml) are harmless and can still be used locally.
  • No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/backend_checks.yml Cleanly removes the Performance-Checks job, its dependency from Backend-Checks-Summary, and its result logging/checking. Also includes a minor cosmetic change from single to double quotes on report_paths.
changelog/7417-remove-drill-tests-from-ci.yaml Standard changelog entry for the removal.
docs/fides/docs/development/development_tips.md Removes the performance_tests nox session bullet and updates drill description to remove CI reference. The drill entry still exists for local use.
noxfiles/DOCKER_TEST_SESSIONS.md Removes the performance_tests row from the Docker test sessions table.
noxfiles/ci_nox.py Removes the performance_tests nox session. The docker_stats and load_tests sessions remain available for local use.

Last reviewed commit: 2088675

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown
Contributor

@adamsachs adamsachs left a comment

Choose a reason for hiding this comment

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

👍 this seems like good cleanup to me.

because i think it's always good to get extra opinions before removing things, i'll ask @nrxsmith to comment on whether he's comfortable removing this path for performance testing and relying more wholesale on his performance testing suite/framework!

@erosselli
Copy link
Copy Markdown
Contributor Author

@greptile re-review. The code is not orphaned since one can still run the load test nox session locally, it's just not part of CI anymore

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@nrxsmith
Copy link
Copy Markdown
Contributor

👍 this seems like good cleanup to me.

because i think it's always good to get extra opinions before removing things, i'll ask @nrxsmith to comment on whether he's comfortable removing this path for performance testing and relying more wholesale on his performance testing suite/framework!

I think this is ok, but I'm curious what y'all think of the endpoints its testing. Looks like:

Name Method Endpoint
Health Check GET /health
Login POST /api/v1/login
Get Privacy-Experience [Basic] GET /api/v1/privacy-experience
Get Privacy-Experience [Include GVL] GET /api/v1/privacy-experience?include_gvl=1
Get Data Use GET /api/v1/data_use
Get Data Category GET /api/v1/data_category
Get Data Subject GET /api/v1/data_subject
Get Systems GET /api/v1/system
Get Datasets GET /api/v1/dataset

Some of these we're testing already, but maybe worth adding some more some of these others? My guess is these are kind of old, so maybe there are better ones to test for performance responses; a backlog thing for me.

@erosselli erosselli added this pull request to the merge queue Feb 19, 2026
Merged via the queue into main with commit 78b46ff Feb 19, 2026
53 checks passed
@erosselli erosselli deleted the erosselli/remove-drill-performance-tests branch February 19, 2026 13:12
jpople pushed a commit that referenced this pull request Feb 23, 2026
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.

3 participants