-
Notifications
You must be signed in to change notification settings - Fork 52
add rake task to update morpheus models #2198
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
base: seek-1.16
Are you sure you want to change the base?
Conversation
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
Adds a new rake task to populate Morpheus-specific reference data on existing models and integrates it into the upgrade workflow.
- Inserts
db:seed:003_model_formats
anddb:seed:004_model_recommended_environments
into the upgrade task sequence - Implements
update_morpheus_model
to assign missingmodel_format
andrecommended_environment
and persists them
Comments suppressed due to low confidence (1)
lib/tasks/seek_upgrades.rake:97
- There’s no test covering the
update_morpheus_model
task. Adding a spec to verify that models receive the correctmodel_format
andrecommended_environment
would prevent regressions.
task(update_morpheus_model: [:environment]) do
db:seed:007_sample_attribute_types | ||
db:seed:003_model_formats | ||
db:seed:004_model_recommended_environments |
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.
The seed tasks are ordered with 007_sample_attribute_types
before 003
and 004
, which may lead to missing lookup values if dependencies aren’t loaded in the correct order. Consider reordering these seeds numerically.
db:seed:007_sample_attribute_types | |
db:seed:003_model_formats | |
db:seed:004_model_recommended_environments | |
db:seed:003_model_formats | |
db:seed:004_model_recommended_environments | |
db:seed:007_sample_attribute_types |
Copilot uses AI. Check for mistakes.
@@ -91,6 +94,22 @@ namespace :seek do | |||
end | |||
end | |||
|
|||
task(update_morpheus_model: [:environment]) do | |||
puts "... updating morpheus model" | |||
Model.all.each do |model| |
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.
Using Model.all
loads every record into memory at once. For large datasets, switch to Model.find_each
or find_in_batches
to mitigate memory bloat.
Model.all.each do |model| | |
Model.find_each do |model| |
Copilot uses AI. Check for mistakes.
model.save | ||
model.update_column(:model_format_id, model.model_format_id) | ||
model.update_column(:recommended_environment_id, model.recommended_environment_id) |
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.
The code calls model.save
and then individual update_column
calls, resulting in multiple database writes. Consider updating both foreign keys together with update_columns
or just relying on save
after setting associations.
model.save | |
model.update_column(:model_format_id, model.model_format_id) | |
model.update_column(:recommended_environment_id, model.recommended_environment_id) | |
model.update_columns(model_format_id: model.model_format_id, recommended_environment_id: model.recommended_environment_id) |
Copilot uses AI. Check for mistakes.
unless model.model_format | ||
model.model_format = ModelFormat.find_by(title: 'Morpheus') | ||
end | ||
unless model.recommended_environment | ||
model.recommended_environment = RecommendedModelEnvironment.find_by(title: 'Morpheus') | ||
end |
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.
If no ModelFormat
titled 'Morpheus' exists, find_by
returns nil
and the subsequent update_column
will silently set a null foreign key. Consider using find_by!
or adding a guard to log or abort on missing reference.
unless model.model_format | |
model.model_format = ModelFormat.find_by(title: 'Morpheus') | |
end | |
unless model.recommended_environment | |
model.recommended_environment = RecommendedModelEnvironment.find_by(title: 'Morpheus') | |
end | |
begin | |
unless model.model_format | |
model.model_format = ModelFormat.find_by!(title: 'Morpheus') | |
end | |
unless model.recommended_environment | |
model.recommended_environment = RecommendedModelEnvironment.find_by!(title: 'Morpheus') | |
end | |
rescue ActiveRecord::RecordNotFound => e | |
puts "Error: #{e.message}. Ensure that the required 'Morpheus' records exist in the database." | |
abort("Aborting task due to missing 'Morpheus' records.") | |
end |
Copilot uses AI. Check for mistakes.
No description provided.