Skip to content

🐛 Fix ZeroDivisionError in test_time_overhead_on_handlers_of_auth_decorators#9046

Closed
GitHK wants to merge 1 commit intoITISFoundation:masterfrom
GitHK:pr-osparc-fix-zero-division-error-really
Closed

🐛 Fix ZeroDivisionError in test_time_overhead_on_handlers_of_auth_decorators#9046
GitHK wants to merge 1 commit intoITISFoundation:masterfrom
GitHK:pr-osparc-fix-zero-division-error-really

Conversation

@GitHK
Copy link
Copy Markdown
Contributor

@GitHK GitHK commented Apr 21, 2026

What do these changes do?

Fixes a flaky ZeroDivisionError in test_time_overhead_on_handlers_of_auth_decorators that occurs when ref_elapsed_ns (the median of public endpoint timings) resolves to 0.

The previous guard max(ref_elapsed_ns, 1) used an integer floor of 1, which does not protect against a float 0.0 from statistics.median. Replaced with max(ref_elapsed_ns, sys.float_info.min) (~2.2e-308), the smallest positive normalized float, guaranteeing the denominator is never zero.

Related issue/s

How to test

Dev-ops

@GitHK GitHK self-assigned this Apr 21, 2026
@GitHK GitHK added the 🤖-automerge marks PR as ready to be merged for Mergify label Apr 21, 2026
@GitHK GitHK added this to the Etna milestone Apr 21, 2026
@GitHK GitHK marked this pull request as ready for review April 21, 2026 09:53
@sonarqubecloud
Copy link
Copy Markdown

@GitHK
Copy link
Copy Markdown
Contributor Author

GitHK commented Apr 21, 2026

@Mergifyio queue

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 21, 2026

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Waiting for:
  • -closed
  • #review-threads-unresolved = 0
  • #review-threads-unresolved=0
All conditions
  • -closed [📌 queue requirement]
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule default]:
      • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
      • #review-threads-unresolved=0
      • #approved-reviews-by >= 2 [🛡 GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
      • #changes-requested-reviews-by=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
      • label!=🤖-do-not-merge
      • label=🤖-automerge
      • any of [🛡 GitHub branch protection]:
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of [🛡 GitHub branch protection]:
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of [🛡 GitHub branch protection]:
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of [🛡 GitHub branch protection]:
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of [🛡 GitHub branch protection]:
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of [🛡 GitHub branch protection]:
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
      • any of [🛡 GitHub branch protection]:
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of [📌 queue requirement]:
    • check-neutral = Mergify Merge Protections
    • check-skipped = Mergify Merge Protections
    • check-success = Mergify Merge Protections

ref_elapsed_ns = statistics.median(public_elapsed_times)
elapsed_ns = statistics.median(admin_elapsed_times)
elapsed_ratio = elapsed_ns / max(ref_elapsed_ns, 1)
elapsed_ratio = elapsed_ns / max(ref_elapsed_ns, sys.float_info.min)
Copy link
Copy Markdown
Member

@pcrespov pcrespov Apr 21, 2026

Choose a reason for hiding this comment

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

Why would denominator max(ref_elapsed_ns, 1) produce a division by zero?

if ref_elapsed_ns==0 then max(ref_elapsed_ns, 1) == 1 and elapsed_ns / max(ref_elapsed_ns, 1) = elapsed_ns

In the description you mention that it has to do with 1 being integer... but i od not undrstand it. You mean it has to be ``max(ref_elapsed_ns, 1.0)`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thisi s a bit strange ...

It seems to me that the code tested is not the same as the one you modify here. In the snapshot below (from the CI ) i do not see the line elapsed_ratio = elapsed_ns / max(ref_elapsed_ns, 1) ...

Image

what am i missing here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You are correct, this was coming form the hot fix branch which does not have this fix implemented. Closing PR since it's useless.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.77%. Comparing base (1a10aac) to head (da65042).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9046      +/-   ##
==========================================
- Coverage   87.35%   82.77%   -4.58%     
==========================================
  Files        2056      792    -1264     
  Lines       81003    37011   -43992     
  Branches     1451      182    -1269     
==========================================
- Hits        70758    30636   -40122     
+ Misses       9834     6325    -3509     
+ Partials      411       50     -361     
Flag Coverage Δ
integrationtests 63.91% <ø> (-0.05%) ⬇️
unittests 86.04% <ø> (-0.19%) ⬇️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 69.39% <ø> (-16.61%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 78.96% <ø> (-12.64%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 73.88% <ø> (-13.62%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 86.78% <ø> (+0.01%) ⬆️

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 1a10aac...da65042. 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.

@GitHK
Copy link
Copy Markdown
Contributor Author

GitHK commented Apr 21, 2026

did not properly check, already fixed, the Hotfix branch was casing issues

@GitHK GitHK closed this Apr 21, 2026
@GitHK GitHK deleted the pr-osparc-fix-zero-division-error-really branch April 21, 2026 10:52
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants