Conversation
Co-authored-by: saratpoluri <1325325+saratpoluri@users.noreply.github.com>
Co-authored-by: saratpoluri <1325325+saratpoluri@users.noreply.github.com>
Co-authored-by: saratpoluri <1325325+saratpoluri@users.noreply.github.com>
Co-authored-by: saratpoluri <1325325+saratpoluri@users.noreply.github.com>
Co-authored-by: saratpoluri <1325325+saratpoluri@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors the Manager (Django) database migration workflow to stop generating migrations at runtime, ensure migrations are version-controlled, and reorganize the Manager Django project layout to match the Docker image structure.
Changes:
- Removes runtime
makemigrationsbehavior and updates startup to apply migrations viamigrateonly. - Adds a developer/release-time migration generation script + migration workflow documentation.
- Restructures Manager Django source tree (templates/static/JS) under
manager/src/manager/and updates build/copy paths accordingly.
Reviewed changes
Copilot reviewed 10 out of 133 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| manager/tools/migration | Marks legacy migration helper as deprecated. |
| manager/tools/migrate-rename-manager | Marks one-time rename helper as deprecated. |
| manager/tools/generate_migrations.sh | Adds container-based migration generation script for dev/release workflow. |
| manager/src/manager/wsgi.py | Adds WSGI entrypoint for Django project. |
| manager/src/manager/validators.py | Adds/relocates file validators (assets, zip datasets, LLA corners). |
| manager/src/manager/urls.py | Adds/relocates URL routing (UI + API endpoints). |
| manager/src/manager/templatetags/user_tags.py | Adds template filters used by templates. |
| manager/src/manager/templates/sscape/singletonArea.html | Adds/relocates template. |
| manager/src/manager/templates/sscape/sign_in.html | Adds/relocates template. |
| manager/src/manager/templates/sscape/index.html | Adds/relocates template. |
| manager/src/manager/templates/sscape/base_3d.html | Adds/relocates base template for 3D view. |
| manager/src/manager/templates/sscape/base.html | Adds/relocates base template. |
| manager/src/manager/templates/sscape/account_locked.html | Adds/relocates template for lockout page. |
| manager/src/manager/templates/sscape/500_error.html | Adds/relocates debug 500 template. |
| manager/src/manager/templates/singleton_sensor/singleton_sensor_update.html | Adds/relocates template. |
| manager/src/manager/templates/singleton_sensor/singleton_sensor_list.html | Adds/relocates template. |
| manager/src/manager/templates/singleton_sensor/singleton_sensor_delete.html | Adds/relocates template. |
| manager/src/manager/templates/singleton_sensor/singleton_sensor_create.html | Adds/relocates template. |
| manager/src/manager/templates/scene/scene_list.html | Adds/relocates template. |
| manager/src/manager/templates/scene/scene_import.html | Adds/relocates template. |
| manager/src/manager/templates/scene/scene_detail.html | Adds/relocates template. |
| manager/src/manager/templates/scene/scene_delete.html | Adds/relocates template. |
| manager/src/manager/templates/scene/geospatial.html | Adds/relocates geospatial demo template. |
| manager/src/manager/templates/model/model_list.html | Adds/relocates model directory UI template. |
| manager/src/manager/templates/model/includes/model_directory.html | Adds/relocates recursive model tree partial. |
| manager/src/manager/templates/child/child_update.html | Adds/relocates template. |
| manager/src/manager/templates/child/child_delete.html | Adds/relocates template. |
| manager/src/manager/templates/child/child_create.html | Adds/relocates template. |
| manager/src/manager/templates/cam/cam_update.html | Adds/relocates template. |
| manager/src/manager/templates/cam/cam_list.html | Adds/relocates template. |
| manager/src/manager/templates/cam/cam_delete.html | Adds/relocates template. |
| manager/src/manager/templates/cam/cam_create.html | Adds/relocates template. |
| manager/src/manager/templates/asset/asset_update.html | Adds/relocates template. |
| manager/src/manager/templates/asset/asset_list.html | Adds/relocates template. |
| manager/src/manager/templates/asset/asset_delete.html | Adds/relocates template. |
| manager/src/manager/templates/asset/asset_create.html | Adds/relocates template. |
| manager/src/manager/static/site.webmanifest | Adds/relocates manifest and updates REUSE path reference. |
| manager/src/manager/static/js/utils.js | Adds shared JS utilities used across UI. |
| manager/src/manager/static/js/toast.js | Adds toast helper used by UI. |
| manager/src/manager/static/js/thing/scenetripwire.js | Adds/relocates tripwire 3D object logic. |
| manager/src/manager/static/js/thing/scenesensor.js | Adds/relocates sensor 3D object logic. |
| manager/src/manager/static/js/thing/managers/tripwiremanager.js | Adds/relocates manager for tripwires. |
| manager/src/manager/static/js/thing/managers/thingmanager.js | Adds/relocates generic “thing” manager (REST-backed). |
| manager/src/manager/static/js/thing/managers/sensormanager.js | Adds/relocates manager for sensors. |
| manager/src/manager/static/js/thing/managers/regionmanager.js | Adds/relocates manager for regions. |
| manager/src/manager/static/js/thing/managers/cameramanager.js | Adds/relocates manager for cameras. |
| manager/src/manager/static/js/thing/controls/validateinputcontrols.js | Adds/relocates input validation helpers. |
| manager/src/manager/static/js/thing/controls/thingtransformcontrols.js | Adds/relocates transform controls behavior. |
| manager/src/manager/static/js/thing/controls/thingcontrols.js | Adds/relocates generic GUI controls for objects. |
| manager/src/manager/static/js/restclient.js | Adds REST client abstraction used by JS UI. |
| manager/src/manager/static/js/pipeline-generator.js | Adds client-side pipeline preview generator workflow. |
| manager/src/manager/static/js/interactions.js | Adds 3D viewport interaction handlers. |
| manager/src/manager/static/js/geospatial/map-interface.js | Adds strategy-pattern base for geospatial providers. |
| manager/src/manager/static/js/geospatial/geomanager.js | Adds geospatial provider switching and orchestration. |
| manager/src/manager/static/js/draw.js | Adds/relocates drawing utilities for 3D scene. |
| manager/src/manager/static/js/customcamerahelper.js | Adds/relocates custom camera helper rendering. |
| manager/src/manager/static/js/constants.js | Adds/relocates shared constants for JS UI. |
| manager/src/manager/static/js/childscene.js | Adds/relocates child-scene form logic. |
| manager/src/manager/static/js/assetmanager.js | Adds/relocates asset loading & tracking mark plotting. |
| manager/src/manager/static/favicon-32x32.png | Adds/relocates static asset. |
| manager/src/manager/static/favicon-16x16.png | Adds/relocates static asset. |
| manager/src/manager/static/css/scenescape.css | Adds/relocates styling for 3D scene UI. |
| manager/src/manager/static/css/images/rotate.svg | Adds/relocates static asset. |
| manager/src/manager/static/css/geospatial.css | Adds/relocates styling for geospatial demo. |
| manager/src/manager/setup.py | Adds packaging metadata for manager module. |
| manager/src/manager/settings_local.py | Adds/relocates local settings overrides. |
| manager/src/manager/settings.py | Adds/relocates Django settings and debug middleware wiring. |
| manager/src/manager/scene_import.py | Adds/relocates scene import implementation. |
| manager/src/manager/ppl_generator/pipeline_generator.py | Adds pipeline generation logic for DLSPS integration. |
| manager/src/manager/ppl_generator/model_chain.py | Adds model-chain parsing/serialization for pipelines. |
| manager/src/manager/ppl_generator/inference_model.py | Adds inference model serialization and validation. |
| manager/src/manager/ppl_generator/config_generator.py | Adds DLSPS config generation wrapper. |
| manager/src/manager/ppl_generator/common_types.py | Adds common types/exceptions for pipeline generation. |
| manager/src/manager/ppl_generator/init.py | Adds module exports + lazy import behavior. |
| manager/src/manager/migrations/init.py | Ensures Django migrations package exists in repo. |
| manager/src/manager/middleware.py | Adds debug-only custom 500 middleware. |
| manager/src/manager/management/commands/updatedbstatus.py | Adds management command to mark DB readiness. |
| manager/src/manager/management/commands/kubecommand.py | Adds management command for kube client container behavior. |
| manager/src/manager/management/commands/db2config.py | Adds management command to export config from DB. |
| manager/src/manager/management/commands/createuser.py | Adds management command to create/update users + ACLs. |
| manager/src/manager/management/commands/init.py | Introduces init file (but currently misnamed). |
| manager/src/manager/management/init.py | Introduces init file (but currently misnamed). |
| manager/src/manager/fields.py | Adds/relocates DB field compatibility layer. |
| manager/src/manager/context_processors.py | Adds context processor for UI templates. |
| manager/src/manager/calculate_intrinsics_view.py | Adds REST endpoint for intrinsics/pose calculation. |
| manager/src/manager/admin.py | Adds admin registration for PubSubACL. |
| manager/src/manager/init.py | Adds/relocates package marker. |
| manager/src/manage.py | Adds/relocates Django manage.py. |
| manager/config/scenescape-init | Updates container init to apply migrations and (optionally) preload example data. |
| manager/Makefile | Updates JS library output directory to new static tree. |
| manager/MIGRATIONS.md | Adds documented migration workflow. |
| manager/Dockerfile | Updates Docker copy layout to new manager/src/manager structure. |
| manager/Agents.md | Updates service guide to reflect new paths + migration doc. |
| REUSE.toml | Updates manifest path after static directory move. |
| .gitignore | Adjusts ignores for runtime migrations marker directory. |
You can also share your feedback on Copilot code review. Take the survey.
| if [ ! -e "${MIGRATIONS_MARKER}" ]; then | ||
| CREATEDB=1 | ||
| mkdir -p "$(dirname "${MIGRATIONS_MARKER}")" | ||
| touch "${MIGRATIONS_MARKER}" | ||
| fi |
There was a problem hiding this comment.
The if [ ! -e "${MIGRATIONS_MARKER}" ] block is missing indentation for its body (CREATEDB/mkdir/touch). Shell will still run, but this makes it easy to misread and is inconsistent with the rest of the script. Indent the body of the if consistently.
| echo "" | ||
| echo "==> Migration files generated in manager/src/django/migrations/" | ||
| echo "" |
There was a problem hiding this comment.
generate_migrations.sh prints an outdated path (manager/src/django/migrations/) even though migrations are copied/generated under manager/src/manager/migrations. Update the message so developers are pointed to the correct directory.
| generate_migrations() { | ||
| local container="scenescape-migrate-gen-$$" | ||
|
|
||
| mkdir -p "${HOST_MIGRATIONS_DIR}" | ||
| docker run --name "${container}" \ | ||
| --user "$(id -u):$(id -g)" \ | ||
| -v "${SECRETS_DIR}/django:/run/secrets/django:ro" \ | ||
| -v "${PROJECT_ROOT}/manager/src/manager/migrations:/home/scenescape/SceneScape/manager/migrations:rw" \ | ||
| -v "${PROJECT_ROOT}/manager/src/manager:/home/scenescape/SceneScape/manager:rw" \ | ||
| --entrypoint /bin/bash \ | ||
| "${IMAGE}" \ | ||
| -lc ' | ||
| set -euo pipefail | ||
| cd /home/scenescape/SceneScape | ||
| cp /run/secrets/django/secrets.py /home/scenescape/SceneScape/manager/secrets.py | ||
| python manage.py makemigrations manager | ||
| ' | ||
| docker rm -f "${container}" >/dev/null | ||
| echo "==> Migrations can be found at ${HOST_MIGRATIONS_DIR}" | ||
| } |
There was a problem hiding this comment.
If the docker run that generates migrations fails, the container named scenescape-migrate-gen-$$ will be left behind because cleanup happens only after a successful run. Add a trap (EXIT/ERR) to always remove the container (and consider removing the temporary secrets copy) to avoid accumulating stopped containers.
| if [ "${CREATEDB}" -eq 1 ] && [ -n "${EXAMPLEDB:-}" ]; then | ||
| echo "==> Preloading example DB..." | ||
| mkdir -p "${DBROOT}/media" | ||
| tar -C "${DBROOT}/media" -xf "${EXAMPLEDB}" | ||
| ./manage.py loaddata "${DBROOT}/media/data.json" | ||
| rm -f "${DBROOT}/media/data.json" "${DBROOT}/media/meta.json" | ||
| fi |
There was a problem hiding this comment.
The new migration preloading block will run on first start when EXAMPLEDB is set, but the legacy preload block later in this script still exists (further down, inside if [ ${CREATEDB} = 1 ]). That results in loaddata/tar extraction being executed twice on first run. Remove the older preload block or consolidate into a single code path.
jakubsikorski
left a comment
There was a problem hiding this comment.
There are errors during build:
ln: failed to create symbolic link 'manager/src/django/': No such file or directory
Updated |
| 2. **Generate migration file** using the generate_migrations.sh script: | ||
|
|
||
| ```bash | ||
| bash manager/tools/generate_migrations.sh |
There was a problem hiding this comment.
On clear repo, without scenescape deployed it is failing. I suggesting adding pre-requirements in readme with minimal requirements.
There was a problem hiding this comment.
I deployed scenescape. But when I run script I got errors about unable to connect to postgres.
==> Django Migration Generator for SceneScape Manager
/usr/local/lib/python3.11/site-packages/django/core/management/commands/makemigrations.py:161: RuntimeWarning: Got an error checking a consistent migration history performed for database connection 'default': connection to server at "localhost" (::1), port 5432 failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?
connection to server at "localhost" (127.0.0.1), port 5432 failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?
warnings.warn(
Intel® SceneScape version.txt file not found.
No changes detected in app 'manager'
==> Migrations can be found at /home/labrat/scenescape-review/manager/src/manager/migrations
==> Migration files generated in manager/src/manager/migrations/| 2. **Generate migration file** using the generate_migrations.sh script: | ||
|
|
||
| ```bash | ||
| bash manager/tools/generate_migrations.sh |
There was a problem hiding this comment.
I deployed scenescape. But when I run script I got errors about unable to connect to postgres.
==> Django Migration Generator for SceneScape Manager
/usr/local/lib/python3.11/site-packages/django/core/management/commands/makemigrations.py:161: RuntimeWarning: Got an error checking a consistent migration history performed for database connection 'default': connection to server at "localhost" (::1), port 5432 failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?
connection to server at "localhost" (127.0.0.1), port 5432 failed: Connection refused
Is the server running on that host and accepting TCP/IP connections?
warnings.warn(
Intel® SceneScape version.txt file not found.
No changes detected in app 'manager'
==> Migrations can be found at /home/labrat/scenescape-review/manager/src/manager/migrations
==> Migration files generated in manager/src/manager/migrations/
📝 Description
Summary
This PR refactors the database migration workflow.
Changes
makemigrationsfrom the web container initialization.python manage.py migrateto apply existing migrations when necessary.manaer/src/manager/migrations/directory, enabling proper migration history and upgrades between releases.manager/src/djangodirectory to better mirror the structure used inside the manager Docker image. This improves developer ergonomics when working with Django files and helps avoid Docker mount and permission issues.Benefits
✨ Type of Change
Select the type of change your PR introduces:
🧪 Testing Scenarios
Describe how the changes were tested and how reviewers can test them too:
✅ Checklist
Before submitting the PR, ensure the following: