Skip to content

Conversation

@pcrespov
Copy link
Member

@pcrespov pcrespov commented Oct 6, 2025

What do these changes do?

This PR introduces a temporary patch to prevent a deployment bug, until #8448 is resolved.

It enforces, in the docker-compose configuration, that the webserver service uses WEBSERVER_RPC_NAMESPACE = DEFAULT_WEBSERVER_RPC_NAMESPACE = webserver, ensuring consistency with the payments and director-v2 services, which communicate using DEFAULT_WEBSERVER_RPC_NAMESPACE.

Additionally, this PR includes tests to validate this condition until #8448 is resolved.

Related issue/s

How to test

cd services/web/server
make install-dev
pytest -vv tests/unit/isolated/test_application_settings.py 

Dev-ops

None

@pcrespov pcrespov added this to the Cheops milestone Oct 6, 2025
@pcrespov pcrespov self-assigned this Oct 6, 2025
@pcrespov pcrespov requested a review from sanderegg as a code owner October 6, 2025 15:04
@pcrespov pcrespov added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Oct 6, 2025
@codecov
Copy link

codecov bot commented Oct 6, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.70%. Comparing base (5199e38) to head (41602e1).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8463      +/-   ##
==========================================
- Coverage   87.72%   87.70%   -0.03%     
==========================================
  Files        1983     1983              
  Lines       77277    77277              
  Branches     1333     1333              
==========================================
- Hits        67790    67773      -17     
- Misses       9088     9105      +17     
  Partials      399      399              
Flag Coverage Δ
integrationtests 64.17% <ø> (+<0.01%) ⬆️
unittests 86.38% <ø> (-0.02%) ⬇️
Components Coverage Δ
pkg_aws_library 93.59% <ø> (ø)
pkg_celery_library 83.76% <ø> (ø)
pkg_dask_task_models_library 79.33% <ø> (ø)
pkg_models_library 93.07% <ø> (ø)
pkg_notifications_library 85.20% <ø> (ø)
pkg_postgres_database 87.95% <ø> (ø)
pkg_service_integration 70.17% <ø> (ø)
pkg_service_library 70.92% <ø> (ø)
pkg_settings_library 90.19% <ø> (ø)
pkg_simcore_sdk 84.95% <ø> (ø)
agent 93.53% <ø> (ø)
api_server 91.78% <ø> (ø)
autoscaling 95.71% <ø> (ø)
catalog 92.36% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 91.78% <ø> (-0.57%) ⬇️
datcore_adapter 97.94% <ø> (ø)
director 75.90% <ø> (ø)
director_v2 90.93% <ø> (-0.02%) ⬇️
dynamic_scheduler 96.71% <ø> (-0.09%) ⬇️
dynamic_sidecar 90.43% <ø> (ø)
efs_guardian 89.74% <ø> (ø)
invitations 91.41% <ø> (ø)
payments 92.71% <ø> (ø)
resource_usage_tracker 92.20% <ø> (+0.10%) ⬆️
storage 86.45% <ø> (-0.09%) ⬇️
webclient ∅ <ø> (∅)
webserver 87.63% <ø> (-0.04%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5199e38...41602e1. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Copy link
Contributor

Copilot AI left a 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 implements a temporary fix to prevent deployment bugs by enforcing consistent WEBSERVER_RPC_NAMESPACE configuration across services. The change ensures that webserver, wb-db-event-listener, and wb-garbage-collector services all use the hardcoded value "webserver" instead of the variable ${WEBSERVER_HOST}, maintaining consistency with payments and director-v2 services.

Key Changes:

  • Hardcoded WEBSERVER_RPC_NAMESPACE to "webserver" in docker-compose configuration for three services
  • Added comprehensive tests to validate RPC namespace configuration requirements across different service types

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
services/docker-compose.yml Updated WEBSERVER_RPC_NAMESPACE from variable to hardcoded "webserver" for consistency
services/web/server/tests/unit/isolated/test_application_settings.py Added test suite to validate RPC namespace requirements across service types

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@pcrespov pcrespov enabled auto-merge (squash) October 6, 2025 15:12
@pcrespov pcrespov added the 🤖-automerge marks PR as ready to be merged for Mergify label Oct 6, 2025
Copy link
Member

@mrnicegyu11 mrnicegyu11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok 👍 🐰 thx

@mergify
Copy link
Contributor

mergify bot commented Oct 6, 2025

🧪 CI Insights

Here's what we observed from your CI run for 41602e1.

🟢 All jobs passed!

But CI Insights is watching 👀

Copy link
Contributor

@wvangeit wvangeit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@pcrespov pcrespov force-pushed the fix/webserver-rpc-namespace branch from 1b7e1cf to 41602e1 Compare October 6, 2025 16:12
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 6, 2025

@pcrespov pcrespov merged commit 266c7c3 into ITISFoundation:master Oct 6, 2025
95 checks passed
@pcrespov pcrespov deleted the fix/webserver-rpc-namespace branch October 6, 2025 16:48
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Oct 31, 2025
47 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants