-
Notifications
You must be signed in to change notification settings - Fork 33
Update index settings before document import #391
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
Merged
norkunas
merged 1 commit into
meilisearch:main
from
norkunas:update-settings-before-import
Aug 5, 2025
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
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.
Catch & surface
SettingsUpdater::update()failures to avoid silent aborts$this->settingsUpdater->update()is now executed before any data is imported.If this call throws (e.g.
TimeOutException, connectivity issues), the whole import command terminates and no documents are indexed, potentially leaving the index empty.Guard the call so the operator at least gets a clear-to-read error and, optionally, can continue with document import:
This mirrors the explicit error handling already present inside
formatIndexingResponse().📝 Committable suggestion
🤖 Prompt for AI Agents
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 think this is for a next PR deciding if partial import, when some indexes in the middle fails is okay 😛
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.
@norkunas Fair point! You're right that this PR is focused on the core timeout fix, and the error handling strategy for partial imports is indeed a separate concern that deserves its own focused discussion.
Would you like me to create a follow-up issue to track the error handling improvement for future consideration?
✏️ Learnings added
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.
Sure
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.
🧩 Analysis chain
I'll create a follow-up issue to track the error handling improvement for settings updates.
@norkunas I've created a follow-up issue to track the error handling improvements for settings updates. The issue captures the context from our discussion and outlines the key considerations around partial import behavior when settings updates fail.
🏁 Script executed:
Length of output: 1281
@norkunas Perfect! I've successfully created issue #392 to track the error handling improvements for settings updates in the MeilisearchImportCommand.
The issue includes:
This keeps the current PR focused on the core timeout fix while ensuring the error handling improvements don't get lost. Thanks for keeping the scope clean! 🎯
🧬 Code Graph Analysis Results
Relevant snippets for understanding
src/Command/MeilisearchImportCommand.phpFile:
src/Model/Aggregator.php(lines 12-80)getEntities()returning array of entity class names aggregated.MeilisearchImportCommand::entitiesToIndex()to expand aggregator entities into their constituent entities.File:
src/Services/SettingsUpdater.php(lines 17-79)MeilisearchImportCommandto update index settings.File:
src/Command/IndexCommand.php(lines 13-60)getEntitiesFromArgs()to get configured indices filtered by CLI option.File:
src/SearchService.php(selected method signatures)getConfiguration()returns configuration collection including indices.isSearchable()checks if a class is searchable.index()indexes entities and returns an array of indexing responses.deleteByIndexName()deletes an index by name.File:
src/Collection.php(selected methods)all(): returns all items as array.forget($keys): removes items by keys.reject($callback): filters out items matching callback.transform($callback): transforms items by callback.firstWhere($key, $operator = null, $value = null): returns first item matching key-value.unique($key = null, bool $strict = false): returns unique items.MeilisearchImportCommandfor index collections.File:
src/EventListener/ConsoleOutputSubscriber.php(lines 11-31)MeilisearchImportCommandto provide console feedback.File:
src/Exception/TaskException.php(lines 7-9)MeilisearchImportCommand::formatIndexingResponse().File:
src/Engine.php(index method, lines 28-58)File:
src/Command/MeilisearchUpdateSettingsCommand.php(lines 19-93)entitiesToIndex()method asMeilisearchImportCommandto expand aggregators.SettingsUpdaterservice to update settings.ConsoleOutputSubscriberfor console feedback.File:
src/Command/MeilisearchCreateCommand.php(lines 20-113)entitiesToIndex().SettingsUpdaterto update settings optionally.ConsoleOutputSubscriberfor console feedback.File:
src/Command/MeilisearchDeleteCommand.php(lines 14-50)SearchService::deleteByIndexName()to delete indices.File:
src/Command/MeilisearchClearCommand.php(lines 12-47)SearchService::clear()method.Summary
The
MeilisearchImportCommandclass is a Symfony console command that imports entities into Meilisearch indices. It uses:SearchServiceto check if entities are searchable and to perform indexing.SettingsUpdaterto update index settings optionally.ConsoleOutputSubscriberto provide console feedback on events.Collectionclass to manage index collections and filtering.Aggregatorabstract class to expand aggregator entities into their constituent entities.Engineclass to perform the actual indexing calls to Meilisearch.TaskExceptionto handle errors during indexing.The command supports options for batch size, skipping batches, response timeout, updating settings, and swapping indices to avoid downtime. It handles batching of entities, waits for indexing tasks to complete, and can swap temporary indices with live ones.
These related classes and methods provide context for how the import command operates, manages entities, updates settings, and interacts with Meilisearch.
🧠 Learnings used