Skip to content

🎨 Require parent info headers when running function #7900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 20 commits into from
Jun 17, 2025
Merged

🎨 Require parent info headers when running function #7900

merged 20 commits into from
Jun 17, 2025

Conversation

@bisgaard-itis
Copy link
Contributor

@bisgaard-itis bisgaard-itis commented Jun 13, 2025 •
edited
Loading

What do these changes do?

  • Require that parent info headers are specified when running function or map via the api-server. I want to require the headers are there, but need to allow them to be null in case there is no parent (e.g. when testing via the swagger view). The way I was able to achieve this was by introduce a NullString string. This is a bit similar to the UnSet from @pcrespov.

Related issue/s

How to test

Dev-ops

@bisgaard-itis bisgaard-itis requested a review from pcrespov as a code owner June 13, 2025 14:39
@bisgaard-itis bisgaard-itis marked this pull request as draft June 13, 2025 14:39
@bisgaard-itis bisgaard-itis self-assigned this Jun 13, 2025
@bisgaard-itis bisgaard-itis added this to the Engage milestone Jun 13, 2025
@codecov
Copy link

codecov bot commented Jun 13, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.56%. Comparing base (5e3f6be) to head (549a572).
Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (5e3f6be) and HEAD (549a572). Click for more details.

HEAD has 30 uploads less than BASE
Flag BASE (5e3f6be) HEAD (549a572)
unittests 31 1
Additional details and impacted files 
@@             Coverage Diff             @@
##           master    #7900       +/-   ##
===========================================
- Coverage   86.44%   69.56%   -16.88%     
===========================================
  Files        1845      795     -1050     
  Lines       71145    36460    -34685     
  Branches     1220      176     -1044     
===========================================
- Hits        61498    25362    -36136     
- Misses       9295    11040     +1745     
+ Partials      352       58      -294
Flag Coverage Δ
integrationtests 64.17% <ø> (-0.07%) ⬇️
unittests 92.31% <100.00%> (+7.30%) ⬆️
Components Coverage Δ
api ∅ <ø> (∅)
pkg_aws_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 76.92% <ø> (-8.13%) ⬇️
agent ∅ <ø> (∅)
api_server 92.31% <100.00%> (+0.45%) ⬆️
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 77.66% <ø> (-13.45%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 88.33% <ø> (-1.77%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 58.90% <ø> (-23.37%) ⬇️

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 5e3f6be...549a572. 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.

@bisgaard-itis bisgaard-itis marked this pull request as ready for review June 16, 2025 12:00
sanderegg
sanderegg approved these changes Jun 16, 2025
View reviewed changes
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

Just wondering about one thing.
If you want to make it required, I think there is a null UUID which is
00000000-0000-0000-0000-000000000000

import uuid
null_uuid = uuid.UUID(int=0)
# or
str(uuid.UUID(int=0))  # '00000000-0000-0000-0000-000000000000'

wvangeit
wvangeit approved these changes Jun 16, 2025
View reviewed changes
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 @bisgaard-itis

@JavierGOrdonnez
Copy link

JavierGOrdonnez commented Jun 16, 2025

@bisgaard-itis I guess this is internal and will be internally handled by calls to Function API? Or do I need to update calls in the frontend?

@bisgaard-itis
Copy link
Contributor Author

bisgaard-itis commented Jun 17, 2025

@bisgaard-itis I guess this is internal and will be internally handled by calls to Function API? Or do I need to update calls in the frontend?

@JavierGOrdonnez This will probably have to be added manually. I suggest we sit down and look at it together in the afternoon. We have a mechanism which adds this automatically when submitting solver/study jobs. But it is done in the wrapper osparc.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 17, 2025

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@bisgaard-itis
Copy link
Contributor Author

bisgaard-itis commented Jun 17, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jun 17, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 9b7bd7c

@bisgaard-itis bisgaard-itis added the 🤖-automerge marks PR as ready to be merged for Mergify label Jun 17, 2025
@mergify mergify bot merged commit 9b7bd7c into ITISFoundation:master Jun 17, 2025
91 of 96 checks passed
@bisgaard-itis bisgaard-itis deleted the 7846-require-parent-info-when-running-function branch June 17, 2025 07:48
@bisgaard-itis bisgaard-itis mentioned this pull request Jun 17, 2025
Merged
This was referenced Jun 17, 2025
Closed
Closed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
Closed
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcrespov pcrespov pcrespov approved these changes

@wvangeit wvangeit wvangeit approved these changes

@sanderegg sanderegg sanderegg approved these changes

@JavierGOrdonnez JavierGOrdonnez Awaiting requested review from JavierGOrdonnez

Assignees

@bisgaard-itis bisgaard-itis

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:apiserver api-server service Meta-modeling

Projects

None yet

Milestone

Engage

Development

Successfully merging this pull request may close these issues.

Wire parent information through function api

5 participants

@bisgaard-itis @JavierGOrdonnez @wvangeit @pcrespov @sanderegg