Skip to content

Conversation

@sanderegg
Copy link
Member

@sanderegg sanderegg commented May 22, 2025

What do these changes do?

@wvangeit discovered an issue where unexpected key-values in outputs.json would fail a run.
This PR fixes it and also improves on the error shown in the UI logs.
This PR also should fix the unreliability in on-demande computational clusters.

Schema Validation Improvements:

  • Updated from_task_output in io.py to filter out any entries in the task output data that are not defined in the schema. This ensures only valid keys are retained.
  • Added a new test test_create_task_output_from_task_ignores_additional_entries to verify that extra keys in task output files are ignored when creating task output data.

Error Handling Enhancements:

  • Modified get_errors in errors.py to improve error message formatting by directly using the exception's string representation.
  • Added handling for UnboundPortError in parse_output_data in dask.py, ensuring unbound ports are properly reported with detailed error messages.

Related issue/s

How to test

Dev-ops

@sanderegg sanderegg self-assigned this May 22, 2025
@sanderegg sanderegg added a:dask-service Any of the dask services: dask-scheduler/sidecar or worker a:director-v2 issue related with the director-v2 service labels May 22, 2025
@sanderegg sanderegg added this to the Bazinga! milestone May 22, 2025
@sanderegg sanderegg marked this pull request as ready for review May 22, 2025 08:38
@sanderegg sanderegg requested review from GitHK and pcrespov as code owners May 22, 2025 08:38
@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

Attention: Patch coverage is 70.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 74.59%. Comparing base (9e9cc95) to head (f9d3a4d).
Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (9e9cc95) and HEAD (f9d3a4d). Click for more details.

HEAD has 18 uploads less than BASE
Flag BASE (9e9cc95) HEAD (f9d3a4d)
unittests 32 17
integrationtests 6 3
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #7724       +/-   ##
===========================================
- Coverage   87.40%   74.59%   -12.81%     
===========================================
  Files        1838     1161      -677     
  Lines       71063    48589    -22474     
  Branches     1201      186     -1015     
===========================================
- Hits        62110    36247    -25863     
- Misses       8625    12298     +3673     
+ Partials      328       44      -284     
Flag Coverage Δ
integrationtests 58.49% <62.50%> (-5.99%) ⬇️
unittests 87.83% <70.00%> (+1.21%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_library ∅ <ø> (∅)
pkg_dask_task_models_library 79.55% <100.00%> (+0.07%) ⬆️
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 65.28% <ø> (-19.79%) ⬇️
agent 96.46% <ø> (ø)
api_server 91.59% <ø> (ø)
autoscaling ∅ <ø> (∅)
catalog 92.70% <ø> (ø)
clusters_keeper 99.25% <ø> (ø)
dask_sidecar 91.67% <ø> (ø)
datcore_adapter 98.12% <ø> (ø)
director 76.78% <ø> (-0.10%) ⬇️
director_v2 85.32% <62.50%> (-5.74%) ⬇️
dynamic_scheduler 96.76% <ø> (ø)
dynamic_sidecar 90.18% <ø> (ø)
efs_guardian 89.79% <ø> (ø)
invitations 93.28% <ø> (ø)
payments 92.63% <ø> (ø)
resource_usage_tracker 89.02% <ø> (ø)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 56.95% <ø> (-28.78%) ⬇️

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 9e9cc95...f9d3a4d. 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
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix

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

@sanderegg sanderegg force-pushed the bugfix/dask/parsing-output-data branch 2 times, most recently from 6a213b0 to 26649b9 Compare May 22, 2025 12:16
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.

thx!

@sanderegg sanderegg force-pushed the bugfix/dask/parsing-output-data branch from 26649b9 to 4ed2b2d Compare May 22, 2025 12:49
@sanderegg sanderegg changed the title 🐛Dask-sidecar: ignore unexpected key-value pairs in outputs json data 🐛Dask-sidecar: ignore unexpected key-value pairs in outputs json data + unreliable computational runs in on-demand clusters May 22, 2025
@sonarqubecloud
Copy link

@sanderegg sanderegg merged commit 6c43a0d into ITISFoundation:master May 22, 2025
75 of 91 checks passed
@sanderegg sanderegg deleted the bugfix/dask/parsing-output-data branch May 22, 2025 13:29
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jun 6, 2025
92 tasks
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:dask-service Any of the dask services: dask-scheduler/sidecar or worker a:director-v2 issue related with the director-v2 service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants