Skip to content

fix: dummy workflow per tenant #4600

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

Kiryous
Copy link
Contributor

@Kiryous Kiryous commented Apr 21, 2025

Closes #4599

Copy link

vercel bot commented Apr 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
keep ⬜️ Ignored (Inspect) Visit Preview Apr 22, 2025 6:51am

Copy link

codecov bot commented Apr 21, 2025

Codecov Report

Attention: Patch coverage is 22.72727% with 51 lines in your changes missing coverage. Please review.

Project coverage is 46.65%. Comparing base (6e71143) to head (cfd0468).
Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
keep/api/core/db_utils.py 20.00% 20 Missing ⚠️
keep/api/routes/workflows.py 0.00% 15 Missing ⚠️
keep/api/core/db.py 26.31% 14 Missing ⚠️
keep/api/models/db/workflow.py 75.00% 1 Missing ⚠️
keep/workflowmanager/workflowstore.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4600       +/-   ##
===========================================
+ Coverage   30.95%   46.65%   +15.69%     
===========================================
  Files          93      164       +71     
  Lines       10534    16894     +6360     
===========================================
+ Hits         3261     7882     +4621     
- Misses       7273     9012     +1739     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Kiryous Kiryous marked this pull request as ready for review April 21, 2025 13:27
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Apr 21, 2025
@Kiryous Kiryous requested a review from shahargl April 21, 2025 13:27
@dosubot dosubot bot added API API related issues Bug Something isn't working labels Apr 21, 2025
@Kiryous
Copy link
Contributor Author

Kiryous commented Apr 21, 2025

@greptileai

Copy link

@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.

PR Summary

This PR addresses a critical multi-tenancy issue by implementing tenant-specific test workflows. Here's a concise summary of the key changes:

  • Added is_test and is_test_run boolean columns to workflow and workflowexecution tables respectively to properly track test workflows per tenant
  • Implemented thread-safe get_or_create_dummy_workflow function in db_utils.py to handle per-tenant test workflow creation
  • Modified workflow queries to filter out test workflows using Workflow.is_test == False condition
  • Added migration script to clean up deprecated system-test-workflow tenant data while maintaining referential integrity
  • Updated test run functionality to use actual workflow IDs when available instead of defaulting to system-wide test workflow

Key points to review:

  • Migration script properly handles cleanup of old data while maintaining FK constraints
  • Thread-safe implementation of get_or_create prevents race conditions
  • Proper tenant isolation is maintained throughout the workflow execution chain
  • Test workflows are now properly scoped to their respective tenants
  • Clear separation between test and production workflows through is_test flag

10 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API related issues Bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: system-test-workflow breaks tenant isolation
1 participant