From 2362e949dd2f5183002831982a8aab6ab929c8a4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 29 Apr 2025 14:11:57 +0200 Subject: [PATCH 1/6] reduces parallel requests --- .../test_studies_dispatcher_studies_access.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py index a049ae257db..6b5554133b2 100644 --- a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py +++ b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py @@ -435,7 +435,7 @@ async def enforce_garbage_collect_guest(uid): @pytest.mark.flaky(max_runs=3) -@pytest.mark.parametrize("number_of_simultaneous_requests", [1, 2, 32]) +@pytest.mark.parametrize("number_of_simultaneous_requests", [1, 2, 16]) async def test_guest_user_is_not_garbage_collected( number_of_simultaneous_requests: int, web_server: TestServer, From 2c6e689f5b695e2cdc0af9eaf84baa803e2ea6fa Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 29 Apr 2025 14:16:34 +0200 Subject: [PATCH 2/6] doc --- .../test_studies_dispatcher_studies_access.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py index 6b5554133b2..e4046463bb9 100644 --- a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py +++ b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py @@ -435,7 +435,17 @@ async def enforce_garbage_collect_guest(uid): @pytest.mark.flaky(max_runs=3) -@pytest.mark.parametrize("number_of_simultaneous_requests", [1, 2, 16]) +@pytest.mark.parametrize( + "number_of_simultaneous_requests", + [ + 1, + 2, + 16, + # NOTE: The number of simultaneous anonymous users is limited by system load. + # A GuestUsersLimitError is raised if user creation exceeds the MAX_DELAY_TO_CREATE_USER threshold. + # This test is flaky due to its dependency on app runtime conditions. Avoid increasing simultaneous requests. + ], +) async def test_guest_user_is_not_garbage_collected( number_of_simultaneous_requests: int, web_server: TestServer, From f2c331ceb21ff0904fee3a69a326c15664799669 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 29 Apr 2025 14:26:07 +0200 Subject: [PATCH 3/6] improves assert --- .../test_studies_dispatcher_studies_access.py | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py index e4046463bb9..4474ae42525 100644 --- a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py +++ b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py @@ -234,9 +234,24 @@ async def _assert_redirected_to_study( "OSPARC-SIMCORE" in content ), f"Expected front-end rendering workbench's study, got {content!s}" - # Expects fragment to indicate client where to find newly created project - m = re.match(r"/study/([\d\w-]+)", response.real_url.fragment) - assert m, f"Expected /study/uuid, got {response.real_url.fragment}" + # First check if the fragment indicates an error + fragment = response.real_url.fragment + error_match = re.match(r"/error", fragment) + if error_match: + # Parse query parameters to extract error details + query_params = urllib.parse.parse_qs( + fragment.split("?", 1)[1] if "?" in fragment else "" + ) + error_message = query_params.get("message", ["Unknown error"])[0] + error_status = query_params.get("status_code", ["Unknown"])[0] + + pytest.fail( + f"Redirected to error page: Status={error_status}, Message={error_message}" + ) + + # Check for study path if not an error + m = re.match(r"/study/([\d\w-]+)", fragment) + assert m, f"Expected /study/uuid, got {fragment}" # Expects auth cookie for current user assert _is_user_authenticated(session) From 0355879514327a5091f41fddfff67e78704f998f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 29 Apr 2025 14:26:59 +0200 Subject: [PATCH 4/6] cleanup --- .../test_studies_dispatcher_studies_access.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py index 4474ae42525..e273fb1090f 100644 --- a/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py +++ b/services/web/server/tests/unit/with_dbs/04/studies_dispatcher/test_studies_dispatcher_studies_access.py @@ -479,9 +479,6 @@ async def test_guest_user_is_not_garbage_collected( async def _test_guest_user_workflow(request_index): print("request #", request_index, "-" * 10) - - # TODO: heartbeat is missing here! - # TODO: reduce GC activation period to 0.1 secs # every guest uses different client to preserve it's own authorization/authentication cookies client: TestClient = await aiohttp_client(web_server) assert client.app @@ -520,5 +517,4 @@ async def _test_guest_user_workflow(request_index): ] await asyncio.gather(*request_tasks) - # and now the garbage collector shall delete our users since we are done... From 49eacc2b3eb31af68c1e16f30f4e04c39ea848a4 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 29 Apr 2025 14:35:45 +0200 Subject: [PATCH 5/6] cleanup --- .github/PULL_REQUEST_TEMPLATE.md | 26 +++++--------------------- docs/devops-checklist.md | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 21 deletions(-) create mode 100644 docs/devops-checklist.md diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index c142fd7d463..4af208c0cd6 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -10,6 +10,7 @@ ⬆️ Upgrade dependencies. 📝 Add or update documentation. 🔨 Add or update development scripts. + ✅ Add, update or pass tests. 🔒️ Fix security issues. ⚠️ Changes in ops configuration etc. are required before deploying. [ Please add a link to the associated ops-issue or PR, such as in https://github.com/ITISFoundation/osparc-ops-environments or https://git.speag.com/oSparc/osparc-infra ] @@ -28,32 +29,15 @@ or from https://gitmoji.dev/ ## Related issue/s - - + ## How to test -## Dev-ops checklist +## Dev-ops -- [ ] No ENV changes or I properly updated ENV ([read the instruction](https://git.speag.com/oSparc/osparc-ops-deployment-configuration/-/blob/configs/README.md?ref_type=heads#how-to-update-env-variables)) +- [ ] No ENV changes or I properly updated ENV (i.e. repo.config) - + diff --git a/docs/devops-checklist.md b/docs/devops-checklist.md new file mode 100644 index 00000000000..e9752dc69cd --- /dev/null +++ b/docs/devops-checklist.md @@ -0,0 +1,15 @@ +# Devops checklist + +- No ENV changes or I properly updated ENV ([read the instruction](https://git.speag.com/oSparc/osparc-ops-deployment-configuration/-/blob/configs/README.md?ref_type=heads#how-to-update-env-variables)) + +- Some checks that might help your code run stable on production, and help devops assess criticality. + - How can DevOps check the health of the service ? + - How can DevOps safely and gracefully restart the service ? + - How and why would this code fail ? + - What kind of metrics are you exposing ? + - Is there any documentation/design specification for the service ? + - How (e.g. through which loglines) can DevOps detect unexpected situations that require escalation to human ? + - What are the resource limitations (CPU, RAM) expected for this service ? + - Are all relevant variables documented and adjustable via environment variables (i.e. no hardcoded magic numbers) ? + +Ref: Modified from https://oschvr.com/posts/what-id-like-as-sre/ From c2f7bf20ff383cbdd5ebb4b37593b83d59963960 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 29 Apr 2025 15:09:03 +0200 Subject: [PATCH 6/6] @sanderegg review:link --- .github/PULL_REQUEST_TEMPLATE.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 4af208c0cd6..08ea869c9b6 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -38,6 +38,7 @@ or from https://gitmoji.dev/ ## Dev-ops -- [ ] No ENV changes or I properly updated ENV (i.e. repo.config) - - +