ITEP-92735: Mapping service should communicate with outside world over proxy#1469
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates SceneScape’s mapping service exposure so external clients access it through the manager’s Apache reverse proxy (instead of directly via :8444), aligning service-to-outside-world communication with the existing proxy pattern used for autocalibration.
Changes:
- Added Apache reverse-proxy routing for the mapping service under
/api/mapping/. - Updated tests, examples, and OpenAPI docs to use proxied URLs (e.g.,
https://localhost/api/mapping/...) and stopped publishing mapping/autocalibration service ports in compose examples. - Adjusted mapping unit test dependencies to include runtime imports needed by mapping modules under test.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/functional/test_auto_calibration.py | Switch autocalibration functional test base URL to go through the web proxy. |
| tests/functional/test_apriltag_registration_update.py | Switch autocalibration registration tests to use the web proxy base URL. |
| tests/compose/mapping.yml | Stop publishing mapping service port to the host (proxy-only access). |
| tests/compose/autocalibration.yml | Stop publishing autocalibration service port to the host (proxy-only access). |
| tests/api/conftest.py | Point MappingClient at the proxied mapping API path. |
| sample_data/docker-compose-dl-streamer-example.yml | Stop publishing mapping service port (proxy-only access). |
| mapping/tools/client_example.py | Update default CLI URL to the proxied mapping endpoint. |
| mapping/tests/requirements-test.txt | Add mapping runtime deps needed for importing modules in tests. |
| manager/config/default-ssl.conf | Add Apache ProxyPass rules for /api/mapping/ to the mapping backend. |
| docs/user-guide/microservices/mapping-service/mapping-service.md | Update mapping API usage URLs to the proxied path. |
| docs/user-guide/microservices/mapping-service/build-from-source.md | Update validation curl URLs to the proxied path. |
| docs/user-guide/microservices/mapping-service/api-docs/mapping-api.yaml | Update OpenAPI servers to reflect proxy access as recommended. |
| docs/user-guide/microservices/auto-calibration/api-reference.md | Update autocalibration base URL to the proxied path. |
| docs/user-guide/microservices/auto-calibration/_assets/autocalibration-api.yaml | Update OpenAPI server URL for proxied autocalibration access. |
6c87ecd to
caa82d5
Compare
- Comment out direct host port exposure (8443, 8444) in test compose files - Update BASE_URL in autocalibration functional tests to route through the Apache proxy at web.scenescape.intel.com instead of directly to autocalibration.scenescape.intel.com:8443 - Update all /v1/ URL paths to /api/v1/autocalibration/ to match the new ProxyPass configuration Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
tests/functional/test_auto_calibration.py:217
- This request URL is built from a module-level constant, so it ignores the functional test CLI option
--weburl(wired intoFunctionalTest.params). Build the URL fromself.params['weburl']to allow running tests against custom endpoints.
def start_calibration(self, image_b64, intrinsics=None):
url = f"{AUTOCALIB_BASE}/cameras/{self.camera_id}/calibration"
payload = {"image": image_b64}
if intrinsics is not None:
payload["intrinsics"] = intrinsics
try:
r = requests.post(url, json=payload, verify=self.rootcert)
2c3d4a5 to
0792cb7
Compare
ltalarcz
left a comment
There was a problem hiding this comment.
LGTM, assuming Autocalibration and Mapping services were manually sanity checked.
6ba8d1a to
9ac9e30
Compare
|
Manual tests run: |
|
@sbelhaik : if RC3 is definitely getting created, then I would like to merge this one. |
|
@saratpoluri |
📝 Description
This PR updates SceneScape’s mapping service exposure so external clients access it through the manager’s Apache reverse proxy (instead of directly via :8444), aligning service-to-outside-world communication with the existing proxy pattern used for autocalibration.
✨ 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:
Manual tests run:
Upload a polycam zip. Successful registration. Auto calibrate a camera. Successful localization and pose returned to UI.
Mapping service called via UI using "Generate Mesh". Mesh looks correct and the camera pose is accurate wrt Mesh.
✅ Checklist
Before submitting the PR, ensure the following: