-
Notifications
You must be signed in to change notification settings - Fork 445
Remove migrations from image entrypoints and fix default random model field values generation #697
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
Remove migrations from image entrypoints and fix default random model field values generation #697
Conversation
Removed `python manage.py makemigrations` from Docker entrypoint to prevent inconsistent or duplicate migration generation during container startup. Previously, every time a new container was built, `makemigrations` regenerated the migration files from scratch. If the previous container’s migrations were not persisted, Django would recreate `0001_initial.py`, ignoring actual DB schema state. This caused issues when existing tables had column changes: Django would not detect diffs and silently skip necessary migrations, leaving the schema out of sync. Thus, migrations should be included in the version control and so that they can be applied but not regenerated in the runtime (inside containers). Note: Ensure all future model changes include corresponding migration files. Signed-off-by: dodo920306 <[email protected]>
Previously, the `default` values for the `name` fields of agents and networks were set using a function with a prefix, assigned directly to the model field. However, because this function was evaluated during migration generation, the same default value was reused for all rows created after the same migration. This also caused Django to detect spurious changes in migrations, as it expected the default value to be fixed. What we actually want is to generate a new name for each individual row when it's saved. To achieve this, we now move the default value logic into the model's `save()` method, so a fresh name is generated at save time only if the field is unset. Signed-off-by: dodo920306 <[email protected]>
|
Please notice that this PR now also rewrites the logix to generate a random name for models. |
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 removes dynamic migration generation from container startup and fixes default model field values by moving the random name generation logic into the model save methods. It ensures that migration files are maintained in version control while updating the name fields for Agent, Network, and Node models.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/api-engine/api/models.py | Removed default random name generation from field declarations; added save methods to generate names when missing. |
| src/api-engine/api/migrations/0002_userprofile_created_at_alter_agent_name_and_more.py | Updated migration to remove default values and apply unique and blank attributes. |
| src/api-engine/api/migrations/0001_initial.py | Initial migration remains with typos in default values (immutable), reflecting historical defaults. |
| build_image/dockerhub/latest/common/api-engine/entrypoint.sh | Removed makemigrations command to prevent runtime migration generation. |
| build_image/docker/common/api-engine/entrypoint.sh | Removed makemigrations command similar to the other entrypoint. |
Comments suppressed due to low confidence (4)
src/api-engine/api/models.py:170
- Ensure that random_name('agent') generates a truly unique value to satisfy the new unique constraint on the Agent name field when a new instance is saved.
unique=True,
src/api-engine/api/models.py:312
- Verify that random_name('network') produces unique values to comply with the unique constraint for the Network model and prevent potential database errors.
unique=True,
src/api-engine/api/models.py:576
- Confirm that random_name(self.type) reliably returns a valid, unique name for Node instances to adhere to the unique constraint on the name field.
if not self.name:
src/api-engine/api/migrations/0001_initial.py:110
- The default value in the initial migration contains a typo ('netowrk'); note that initial migrations are immutable, but verify if this is acceptable for legacy data.
('name', models.CharField(default='netowrk-30858a0cf2f54ab7a5e16a94958758b5', help_text='network name, can be generated automatically.', max_length=64)),
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.
very valuable pr. LGTM. @yeasy @dodo920306
Removed
python manage.py makemigrationsfrom Docker entrypoint to prevent inconsistent or duplicate migration generation during container startup.Previously, every time a new container was built,
makemigrationsregenerated the migration files from scratch. If the previous container’s migrations were not persisted, Django would recreate0001_initial.py, ignoring actual DB schema state. This caused issues when existing tables had column changes: Django would not detect diffs and silently skip necessary migrations, leaving the schema out of sync.Thus, migrations should be included in the version control and so that they can be applied but not regenerated in the runtime (inside containers).
Note: Ensure all future model changes include corresponding migration files.