Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented Apr 9, 2025

What do these changes do?

This pull request includes several changes aimed at improving the robustness and functionality of the WebSocket handling in the pytest-simcore package, as well as adding new utility functions for service endpoint checking. The most important changes include renaming the RestartableWebSocket class to RobustWebSocket, adding retry logic for WebSocket reconnections, and introducing new functions to check and wait for service endpoints.

Improvements to WebSocket handling:

  • Renamed the RestartableWebSocket class to RobustWebSocket and added the auto_reconnect attribute to enable automatic reconnections.
  • Added retry logic for the _attempt_reconnect method using the tenacity library to handle reconnections more robustly.
  • Updated logging messages to provide clearer information when WebSocket connections are closed or failed. [1] [2]
  • added test_robust_websocket.py test to test the RobustWebSocket
  • pytest-simcore can be installed after using uv init and adding some missing libraries (currently only for that test), note that the pytest-simcore library is not CI tested.

Service endpoint checking:

  • Introduced the _get_service_url and _check_service_endpoint functions to validate and check the readiness of service endpoints. [1] [2]
  • Added the wait_for_service_endpoint_responding function to emulate frontend polling until the service endpoint responds with a 2xx/3xx status code.
  • Removed the API requests from inside the SocketIONodeProgressCompleteWaiter as this is incompatible with fast returning function called in separate threads

Code cleanup and refactoring:

  • Removed the assert_service_ready method and related attributes from the SocketIONodeProgressCompleteWaiter class, as the new service endpoint checking functions handle these checks.
  • Updated various functions to use the new RobustWebSocket class and the new service endpoint checking logic. [1] [2]

Related issue/s

How to test

Dev-ops checklist

@sanderegg sanderegg added the e2e Bugs found by or related to the end-2-end testing label Apr 9, 2025
@sanderegg sanderegg added this to the Pauwel Kwak milestone Apr 9, 2025
@sanderegg sanderegg self-assigned this Apr 9, 2025
@codecov
Copy link

codecov bot commented Apr 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.46%. Comparing base (bb3b81c) to head (efaf41e).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7500      +/-   ##
==========================================
+ Coverage   87.44%   87.46%   +0.01%     
==========================================
  Files        1741     1734       -7     
  Lines       67375    67185     -190     
  Branches     1142     1142              
==========================================
- Hits        58917    58763     -154     
+ Misses       8138     8104      -34     
+ Partials      320      318       -2     
Flag Coverage Δ
integrationtests 65.16% <ø> (-0.01%) ⬇️
unittests 86.65% <ø> (+0.03%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library 93.91% <ø> (ø)
pkg_dask_task_models_library 97.09% <ø> (ø)
pkg_models_library 91.94% <ø> (ø)
pkg_notifications_library 85.26% <ø> (ø)
pkg_postgres_database 88.18% <ø> (ø)
pkg_service_integration 70.03% <ø> (ø)
pkg_service_library 72.86% <ø> (+0.03%) ⬆️
pkg_settings_library 90.79% <ø> (ø)
pkg_simcore_sdk 85.40% <ø> (ø)
agent 96.46% <ø> (ø)
api_server 90.02% <ø> (ø)
autoscaling 96.08% <ø> (ø)
catalog 91.92% <ø> (ø)
clusters_keeper 99.24% <ø> (ø)
dask_sidecar 91.29% <ø> (ø)
datcore_adapter 98.12% <ø> (ø)
director 76.78% <ø> (ø)
director_v2 91.27% <ø> (-0.02%) ⬇️
dynamic_scheduler 97.35% <ø> (ø)
dynamic_sidecar 90.11% <ø> (ø)
efs_guardian 89.79% <ø> (ø)
invitations 93.28% <ø> (ø)
payments 92.66% <ø> (ø)
resource_usage_tracker 89.12% <ø> (-0.11%) ⬇️
storage 87.66% <ø> (-0.11%) ⬇️
webclient ∅ <ø> (∅)
webserver 85.87% <ø> (-0.02%) ⬇️

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 bb3b81c...efaf41e. 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.

@sanderegg sanderegg force-pushed the maintenance/playwright-improvements branch from 44f264a to 0dd64f9 Compare April 10, 2025 15:11
@sanderegg sanderegg marked this pull request as ready for review April 10, 2025 15:12
@sanderegg sanderegg requested review from GitHK, Copilot and odeimaiz April 10, 2025 15:17
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.

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

@sanderegg sanderegg requested a review from bisgaard-itis April 10, 2025 15:19
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 a lot 🥇

Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

Congratulations 🥳

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

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

paired review. great job. Left some comments

@sonarqubecloud
Copy link

@sanderegg sanderegg merged commit cc560d3 into ITISFoundation:master Apr 10, 2025
94 checks passed
@sanderegg sanderegg deleted the maintenance/playwright-improvements branch April 10, 2025 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e Bugs found by or related to the end-2-end testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants