-
Notifications
You must be signed in to change notification settings - Fork 246
feat(compass-collection): Mock Data Generator Script Output Screen QA Items - CLOUDP-356792 #7567
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
Conversation
| In the directory that you created, create a file named | ||
| mockdatascript.js (or any name you'd like). | ||
| In the directory that you created, create a file named{' '} | ||
| <strong>mockdatascript.js</strong> (or any name you'd like). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bolding this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the mock data generator script in Compass by adding batched insertions, improved logging, and version pinning. The changes address several QA items including specifying Faker v9.9.0, implementing batch processing for better performance with large document counts, and adding detailed progress logging.
Key changes:
- Pin Faker installation to version 9.9.0 to ensure consistency
- Implement batched insertions (1000 documents per batch) to improve performance and memory usage
- Add comprehensive logging for generation progress, timing, and completion status
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| script-screen.tsx | Updated installation command to specify Faker v9.9.0 and added strong emphasis to filename in instructions |
| script-generation-utils.ts | Implemented batch insertion logic with 1000 document batches and added detailed console logging throughout the generation process |
Comments suppressed due to low confidence (2)
packages/compass-collection/src/components/mock-data-generator-modal/script-generation-utils.ts:1
- The log message 'Batch inserted successfully' doesn't include information about which batch was inserted or how many documents it contained. Consider adding batch number and size details for better debugging and progress tracking, e.g.,
console.log(\Batch ${Math.floor(batchStart / BATCH_SIZE) + 1}/${numBatches} inserted successfully (${insertResult.insertedCount} documents).`);`
import type { FakerFieldMapping } from './types';
packages/compass-collection/src/components/mock-data-generator-modal/script-generation-utils.ts:1
- The final summary log is missing information about the total number of documents successfully inserted. Consider adding a log line like
console.log(\Total documents inserted: ${TOTAL_DOCUMENTS}`);` to confirm the operation completed as expected.
import type { FakerFieldMapping } from './types';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/compass-collection/src/components/mock-data-generator-modal/script-generation-utils.ts
Outdated
Show resolved
Hide resolved
| } | ||
| > | ||
| npm install @faker-js/faker | ||
| npm install @faker-js/faker@9.9.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is to avoid breaking changes, can we do just major to allow future fixes?
| npm install @faker-js/faker@9.9.0 | |
| npm install @faker-js/faker@9 |
paula-stacho
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small suggestion, otherwise looks good!
Description
Checklist
Motivation and Context
Types of changes