Skip to content

Conversation

@wvangeit
Copy link
Contributor

@wvangeit wvangeit commented Sep 8, 2025

What do these changes do?

While using the caching ability of function job status/output, there was an issue when a user only has read permission for function jobs. The repository layer was refusing to update the status/output coming from the comp backend in the db, used as cache, since the user didn't have write permission. I added an option to the update call to ignore the write permissions (the user can't control what is being written, the values are always coming from the comp backend)

Related issue/s

How to test

Give a user read-only access to a function job. Let user get status/output of the function job. This should succeed now.

Dev-ops

No changes

@wvangeit wvangeit added this to the Cheops milestone Sep 8, 2025
@wvangeit wvangeit self-assigned this Sep 8, 2025
@wvangeit wvangeit added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Sep 8, 2025
@wvangeit wvangeit requested a review from pcrespov as a code owner September 8, 2025 11:42
@wvangeit wvangeit added the a:apiserver api-server service label Sep 8, 2025
@codecov
Copy link

codecov bot commented Sep 8, 2025

Codecov Report

❌ Patch coverage is 80.76923% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.69%. Comparing base (9ad66db) to head (c68204c).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8325      +/-   ##
==========================================
+ Coverage   85.12%   87.69%   +2.57%     
==========================================
  Files        1825     1520     -305     
  Lines       70906    62779    -8127     
  Branches     1311      658     -653     
==========================================
- Hits        60362    55057    -5305     
+ Misses      10149     7489    -2660     
+ Partials      395      233     -162     
Flag Coverage Δ
integrationtests 64.06% <42.85%> (+0.93%) ⬆️
unittests 86.09% <80.76%> (+1.57%) ⬆️
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 71.08% <0.00%> (-0.05%) ⬇️
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 85.03% <ø> (ø)
agent 93.53% <ø> (ø)
api_server 91.91% <ø> (∅)
autoscaling 95.77% <ø> (ø)
catalog 92.34% <ø> (ø)
clusters_keeper 99.13% <ø> (ø)
dask_sidecar 92.37% <ø> (ø)
datcore_adapter 97.94% <ø> (ø)
director 75.81% <ø> (ø)
director_v2 91.02% <ø> (+5.56%) ⬆️
dynamic_scheduler 96.27% <ø> (ø)
dynamic_sidecar 90.46% <ø> (ø)
efs_guardian 89.62% <ø> (ø)
invitations 91.44% <ø> (ø)
payments 92.61% <ø> (ø)
resource_usage_tracker 92.34% <ø> (+0.26%) ⬆️
storage 86.57% <ø> (+0.20%) ⬆️
webclient ∅ <ø> (∅)
webserver 88.00% <100.00%> (+5.97%) ⬆️

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 9ad66db...c68204c. 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.

@mergify
Copy link
Contributor

mergify bot commented Sep 8, 2025

🧪 CI Insights

Here's what we observed from your CI run for c68204c.

🟢 All jobs passed!

But CI Insights is watching 👀

@wvangeit wvangeit requested a review from pcrespov September 8, 2025 18:31
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!

@wvangeit wvangeit added the 🤖-automerge marks PR as ready to be merged for Mergify label Sep 8, 2025
@wvangeit
Copy link
Contributor Author

wvangeit commented Sep 8, 2025

@mergify queue

@mergify
Copy link
Contributor

mergify bot commented Sep 8, 2025

queue

🛑 Configuration not compatible with a branch protection setting

The branch protection setting Require branches to be up to date before merging is not compatible with max_parallel_checks>1, queue_conditions != merge_conditions and must be unset.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 8, 2025

@wvangeit wvangeit merged commit 143120f into ITISFoundation:master Sep 8, 2025
94 of 95 checks passed
@wvangeit wvangeit deleted the fix_status_output_update_permissions branch September 8, 2025 20:09
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 a:apiserver api-server service a:webserver webserver's codebase. Assigning the area is particularly useful for bugs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants