[CHORE] Reimplement fork backstop #4511
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
This minor PR adds a defensive backstop to prevent excessive forking of collections. The change implements a simple check that returns an error if a collection's dependencies exceed 1,000,000. This summary was automatically generated by @propel-code-bot |
![propel-code-bot[bot]](https://avatars.githubusercontent.com/in/1179004?s=60&v=4){kind=small}
|
|
||
| // Defensive backstop to prevent too many forks | ||
| if len(lineageFile.Dependencies) > 1000000 { | ||
| return common.ErrCollectionTooManyFork |
There was a problem hiding this comment.
[BestPractice]
The error name ErrCollectionTooManyFork doesn't follow the common Go naming convention for error variables, which typically use plural form for collections. Consider renaming it to ErrCollectionTooManyForks for consistency and clarity.
| // Defensive backstop to prevent too many forks | ||
| if len(lineageFile.Dependencies) > 1000000 { | ||
| return common.ErrCollectionTooManyFork |
There was a problem hiding this comment.
[BestPractice]
Consider making the hard-coded limit of 1,000,000 forks configurable through a constant variable at the package level. This would improve maintainability and make it easier to adjust the limit in the future without modifying the function code.
| // Defensive backstop to prevent too many forks | |
| if len(lineageFile.Dependencies) > 1000000 { | |
| return common.ErrCollectionTooManyFork | |
| // Defensive backstop to prevent too many forks | |
| if len(lineageFile.Dependencies) > maxAllowedForks { | |
| return common.ErrCollectionTooManyFork | |
| } |
⚡ 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.
## Description of changes _Summarize the changes made by this PR._ - Improvements & Bug fixes - ... - New functionality - Reimplements a backstop to prevent too many forks on a collection ## Test plan _How are these changes tested?_ - [ ] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## 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)?_
Description of changes
Summarize the changes made by this PR.
Test plan
How are these changes tested?
pytestfor python,yarn testfor js,cargo testfor rustDocumentation Changes
Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?