[ENH]: Add models and migration for Create/Delete/Get Tasks (#5546) #5573

tanujnay112 · 2025-10-08T08:20:40Z

Description of changes

Summarize the changes made by this PR.

This is a redo of this change which inadvertently merge conflicted with another change when it landed. That change generated task operator constants in rust by moving the go operator constants file into each service and manually generating a corresponding rust file during each docker containers build phase. This would've required every new service to make sure to copy in this Go file during build even if it didn't otherwise need Go code.

This diff changes that by having a contributor manually generate said constants in rust using a supplied script to avoid the above logistics. There is a rust unit test to make sure the generated constants are in sync with what is prepopulated in the SysDB operators table.

Improvements & Bug fixes
- ^^^
New functionality
- ...

Test plan

How are these changes tested?

Tests pass locally with pytest for python, yarn test for js, cargo test for rust

Migration plan

Are there any migrations, or any forwards/backwards compatibility changes needed in order to make sure this change deploys reliably?

Observability plan

What is the plan to instrument and monitor this change?

Documentation Changes

Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the _docs section?_

## Description of changes _Summarize the changes made by this PR._ - Improvements & Bug fixes - Added db models and SysDB routes to Create, Delete and Get Tasks. - New functionality - ^^^ `CreateTask` here does not kick off the requisite backfill task runs in this change. That is left as followup work. ## Test plan Tests have been added in `go/pkg/sysdb/metastore/db/dao/task_test.go`. - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Migration plan _Are there any migrations, or any forwards/backwards compatibility changes needed in order to make sure this change deploys reliably?_ ## Observability plan _What is the plan to instrument and monitor this change?_ ## Documentation Changes _Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the_ [_docs section](https://github.com/chroma-core/chroma/tree/main/docs/docs.trychroma.com)?_

github-actions · 2025-10-08T08:21:00Z

tanujnay112 · 2025-10-08T08:21:01Z

[ENH]: Add models and migration for Create/Delete/Get Tasks (#5546) #5573 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

propel-code-bot · 2025-10-08T08:26:27Z

Add Task, Operator Models and Migrations for Create/Delete/Get Tasks API with Cross-Language Constant Sync

This PR introduces models, migrations, and service implementations for creating, retrieving, and deleting tasks in the system database (SysDB), along with robust support for operators as a separate entity. It also adds infrastructure and scripting to ensure that operator constants (UUIDs and names) are consistently defined and synced across Go, database (migrations), and Rust source code, with mechanisms and CI tests that validate this invariance. The Rust and Go codebases, protocol buffers, migrations, tests, and supporting scripts have been updated or created to support this new Task/Operator semantic, enforcement, and cross-language compatibility.

Key Changes

• Added new Task and Operator models in go/pkg/sysdb/metastore/db/dbmodel/task.go and go/pkg/sysdb/metastore/db/dbmodel/operator.go.
• Introduced SQL migrations (20251001073000.sql) creating tasks, operators, and task_templates tables, including required indexes, uniqueness, and constraints.
• Implemented operator constant sync: Go (constants.go), database migration, and Rust (rust/types/src/operators_generated.rs) constants kept in sync via code generation script.
• Added full CRUD gRPC/proto API in coordinator.proto for tasks and operators, with implementations in Go service/controllers and glue in Rust (for integration/unit testing).
• Extended the meta domain, DAO, and coordinator/service layers for Task and Operator including insert, get-by-name, soft delete, and get-all for operators.
• Comprehensive tests in Go (task/DAO test suite) and Rust (integration tests) to verify CRUD operations and constant sync, plus test CI enforcement.
• Modified generated mock interfaces to include new APIs for operators and tasks (mocks/ITaskDb.go, mocks/IOperatorDb.go, mocks/IMetaDomain.go).
• Updated core database initializer and migration routines in Go to recognize the new tables.
• Added operator constant generation Rust tooling and documentation (operator_codegen.rs, README_OPERATORS.md).
• Set up a protocol for future operator/task feature expansion in both Go and Rust.

Affected Areas

• go/pkg/sysdb/metastore/db/dbmodel/task.go
• go/pkg/sysdb/metastore/db/dbmodel/operator.go
• go/pkg/sysdb/metastore/db/dao/task.go
• go/pkg/sysdb/metastore/db/dao/operator.go
• go/pkg/sysdb/metastore/db/dao/common.go
• go/pkg/sysdb/metastore/db/dbmodel/constants.go
• go/pkg/sysdb/metastore/db/dbcore/core.go
• go/pkg/sysdb/metastore/db/dbmodel/common.go
• go/pkg/sysdb/coordinator/task.go
• go/pkg/sysdb/grpc/task_service.go
• idl/chromadb/proto/coordinator.proto
• go/pkg/sysdb/metastore/db/migrations/20251001073000.sql
• rust/types/src/operators.rs, rust/types/src/operators_generated.rs, rust/types/operator_codegen.rs, and related Rust changes
• Mock and test files for DB interfaces
• Task and operator integration-testing suites in Go and Rust

This summary was automatically generated by @propel-code-bot

propel-code-bot · 2025-10-08T08:32:24Z

go/pkg/sysdb/coordinator/task.go

+	// Debug logging
+	log.Info("Found task", zap.String("task_id", task.ID.String()), zap.String("name", task.Name), zap.String("input_collection_id", task.InputCollectionID), zap.String("output_collection_name", task.OutputCollectionName))


[BestPractice]

The comment indicates this is for debug logging, but it's using log.Info. To avoid potentially noisy logs in a production environment for a read operation, it would be more appropriate to use log.Debug to align with the stated intent.

Suggested change

// Debug logging

log.Info("Found task", zap.String("task_id", task.ID.String()), zap.String("name", task.Name), zap.String("input_collection_id", task.InputCollectionID), zap.String("output_collection_name", task.OutputCollectionName))

// Debug logging

log.Debug("Found task", zap.String("task_id", task.ID.String()), zap.String("name", task.Name), zap.String("input_collection_id", task.InputCollectionID), zap.String("output_collection_name", task.OutputCollectionName))

⚡ Committable suggestion

Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Context for Agents

[**BestPractice**] The comment indicates this is for debug logging, but it's using `log.Info`. To avoid potentially noisy logs in a production environment for a read operation, it would be more appropriate to use `log.Debug` to align with the stated intent. ```suggestion // Debug logging log.Debug("Found task", zap.String("task_id", task.ID.String()), zap.String("name", task.Name), zap.String("input_collection_id", task.InputCollectionID), zap.String("output_collection_name", task.OutputCollectionName)) ``` ⚡ **Committable suggestion** Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. File: go/pkg/sysdb/coordinator/task.go Line: 172

propel-code-bot · 2025-10-08T08:32:25Z

go/pkg/sysdb/coordinator/task.go

+		deleteCollection := &model.DeleteCollection{
+			ID:           collectionUUID,
+			TenantID:     task.TenantID,
+			DatabaseName: task.DatabaseID,


[BestPractice]

There appears to be a naming inconsistency here. You are assigning task.DatabaseID to the DatabaseName field of model.DeleteCollection. This could be confusing for future readers, as it seems an ID is being stored in a field named for a name. If SoftDeleteCollection indeed expects a database ID, consider renaming the field in model.DeleteCollection to DatabaseID for better clarity, if possible within the scope of this change.

Context for Agents

[**BestPractice**] There appears to be a naming inconsistency here. You are assigning `task.DatabaseID` to the `DatabaseName` field of `model.DeleteCollection`. This could be confusing for future readers, as it seems an ID is being stored in a field named for a name. If `SoftDeleteCollection` indeed expects a database ID, consider renaming the field in `model.DeleteCollection` to `DatabaseID` for better clarity, if possible within the scope of this change. File: go/pkg/sysdb/coordinator/task.go Line: 216

tanujnay112 · 2025-10-08T08:54:44Z

Merge activity

Oct 8, 8:54 AM UTC: @tanujnay112 merged this pull request with Graphite.

rescrv

We could save the operator codegen and probably use protobuf enums, right?

rescrv · 2025-10-08T14:45:51Z

go/pkg/sysdb/coordinator/task.go

+			DatabaseName: task.DatabaseID,
+		}
+
+		err = s.SoftDeleteCollection(ctx, deleteCollection)


Is there not a way to do this transactionally?

tanujnay112 requested a review from rescrv October 8, 2025 08:21

tanujnay112 marked this pull request as ready for review October 8, 2025 08:25

propel-code-bot bot reviewed Oct 8, 2025

View reviewed changes

tanujnay112 merged commit 94817fb into main Oct 8, 2025
62 checks passed

rescrv reviewed Oct 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[ENH]: Add models and migration for Create/Delete/Get Tasks (#5546) #5573

[ENH]: Add models and migration for Create/Delete/Get Tasks (#5546) #5573

Uh oh!

tanujnay112 commented Oct 8, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Oct 8, 2025

Uh oh!

tanujnay112 commented Oct 8, 2025

Uh oh!

propel-code-bot bot commented Oct 8, 2025

Uh oh!

propel-code-bot bot Oct 8, 2025

Uh oh!

propel-code-bot bot Oct 8, 2025

Uh oh!

Uh oh!

tanujnay112 commented Oct 8, 2025

Uh oh!

rescrv left a comment

Uh oh!

rescrv Oct 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// Debug logging
		log.Info("Found task", zap.String("task_id", task.ID.String()), zap.String("name", task.Name), zap.String("input_collection_id", task.InputCollectionID), zap.String("output_collection_name", task.OutputCollectionName))

[ENH]: Add models and migration for Create/Delete/Get Tasks (#5546) #5573

[ENH]: Add models and migration for Create/Delete/Get Tasks (#5546) #5573

Uh oh!

Conversation

tanujnay112 commented Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Test plan

Migration plan

Observability plan

Documentation Changes

Uh oh!

github-actions bot commented Oct 8, 2025

Reviewer Checklist

Testing, Bugs, Errors, Logs, Documentation

System Compatibility

Quality

Uh oh!

tanujnay112 commented Oct 8, 2025

Uh oh!

propel-code-bot bot commented Oct 8, 2025

Uh oh!

propel-code-bot bot Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tanujnay112 commented Oct 8, 2025

Merge activity

Uh oh!

rescrv left a comment

Choose a reason for hiding this comment

Uh oh!

rescrv Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tanujnay112 commented Oct 8, 2025 •

edited

Loading