Skip to content
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.

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

feat: add new endpoints to change log retention for exps and trials [DET-9995] #8982

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

salonig23
Copy link
Contributor

@salonig23 salonig23 commented Mar 8, 2024

Description

Added new API Endpoints, CLI commands and WebUI elements to change log retention for experiments and trials

Test Plan

  • Added integration tests to test the API

For CLI -
Experiment:

  1. Create an experiment.
  2. Wait for the experiment to complete.
  3. Run det e set log-retention-days <exp-id> --days 50
  4. Check the tasks table in the database to see if the associated tasks for the experiment have updated the log retention days to 50.
  5. Run det e set log-retention-days <exp-id> --days none --forever
  6. Check the tasks table in the database to see if the associated tasks for the experiment have updated the log retention days to -1.

Trial:

  1. Create an experiment.
  2. Wait for the experiment to complete.
  3. Run det t set log-retention-days <trial-id> --days 50 for any trial in the experiment.
  4. Check the tasks table in the database to see if the associated tasks for the trial have updated the log retention days to 50.
  5. Run det t set log-retention-days <trial-id> --days none --forever
  6. Check the tasks table in the database to see if the associated tasks for the trial have updated the log retention days to -1.

For API -

  1. Create an experiment.
  2. Wait for the experiment to complete.
  3. Use the following API endpoint:

curl -X 'PUT'
'http://localhost:8080/api/v1/experiments/1/retain_logs'
-H 'accept: application/json'
-H 'Content-Type: application/json'
-d '{
"experimentId": 1,
"numDays": 40
}'

Check the tasks table in the database to see if the associated tasks for the experiment have updated the log retention days to 40.

curl -X 'PUT'
'http://localhost:8080/api/v1/trials/1/retain_logs'
-H 'accept: application/json'
-H 'Content-Type: application/json'
-d '{
"trialId": 1,
"numDays": -1
}'

Check the tasks table in the database to see if the associated tasks for the trial have updated the log retention days to -1.

For WebUI -

  1. Create a few experiments.
  2. Go to the Experiments list in the Web UI and select a few of them and then click on the actions button.
    EC0E20C0-1338-4317-BDCC-F7F637E8A2D7
  3. Select the Retain Logs option from the drop down.
  4. Type any number in the Number of days field when prompted.
  5. Wait for a success message.
  6. Check the tasks table in the database to see if the associated tasks for the experiment have updated the log retention days to the number you put in.
  7. Select a few more experiments and follow the above steps, but this time click on the 'Forever' checkbox instead.
  8. Wait for a success message.
  9. Check the tasks table in the database to see if the associated tasks for the experiment have updated the log retention days to -1.
  10. Also check the Trial List Table to see if the Log Retention Days Column shows up and has the correct days.

Commentary (optional)

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

DET-9995

@cla-bot cla-bot bot added the cla-signed label Mar 8, 2024
Copy link

codecov bot commented Mar 8, 2024

Codecov Report

Attention: Patch coverage is 37.27455% with 313 lines in your changes are missing coverage. Please review.

Project coverage is 47.12%. Comparing base (53bf20e) to head (c2f5e39).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8982      +/-   ##
==========================================
- Coverage   52.04%   47.12%   -4.93%     
==========================================
  Files         750     1153     +403     
  Lines       80799   142358   +61559     
  Branches        0     2421    +2421     
==========================================
+ Hits        42053    67082   +25029     
- Misses      38746    75086   +36340     
- Partials        0      190     +190     
Flag Coverage Δ
backend 42.92% <14.28%> (-0.13%) ⬇️
harness 64.24% <37.37%> (+2.25%) ⬆️
web 38.95% <44.70%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
harness/determined/common/experimental/trial.py 55.60% <100.00%> (+0.21%) ⬆️
master/internal/trial.go 41.69% <100.00%> (+0.11%) ⬆️
master/test/olddata/pre_run_trials.go 100.00% <100.00%> (ø)
...ages/ExperimentDetails/ExperimentDetailsHeader.tsx 74.93% <100.00%> (ø)
...pages/ExperimentDetails/ExperimentTrials.table.tsx 99.00% <100.00%> (ø)
webui/react/src/services/apiConfig.ts 72.70% <100.00%> (ø)
webui/react/src/types.ts 99.66% <100.00%> (ø)
webui/react/src/utils/experiment.ts 83.66% <100.00%> (ø)
master/pkg/model/experiment.go 13.93% <0.00%> (-0.06%) ⬇️
webui/react/src/services/decoder.ts 19.93% <0.00%> (ø)
... and 15 more

... and 415 files with indirect coverage changes

@salonig23 salonig23 changed the title feat: add new endpoints to change log retention for experiments and t… feat: add new endpoints to change log retention for exps and trials Mar 9, 2024
@salonig23 salonig23 changed the title feat: add new endpoints to change log retention for exps and trials feat: add new endpoints to change log retention for exps and trials [DET-9995] Mar 9, 2024
@salonig23 salonig23 marked this pull request as ready for review March 9, 2024 00:17
@salonig23 salonig23 requested review from a team as code owners March 9, 2024 00:17
@salonig23 salonig23 requested review from julian-determined-ai, MikhailKardash, erikwilson and stoksc and removed request for a team March 9, 2024 00:17
master/internal/api_experiment.go Outdated Show resolved Hide resolved
harness/determined/cli/experiment.py Outdated Show resolved Hide resolved
@salonig23 salonig23 assigned erikwilson and unassigned stoksc Mar 11, 2024
@salonig23 salonig23 force-pushed the retain-logs-cli branch 2 times, most recently from 6d62447 to 0d358a3 Compare March 11, 2024 18:11
@salonig23 salonig23 force-pushed the retain-logs-cli branch 2 times, most recently from d4feacb to 1c5a684 Compare March 13, 2024 00:00
@salonig23 salonig23 requested a review from a team as a code owner March 21, 2024 18:24
@salonig23 salonig23 force-pushed the retain-logs-cli branch 2 times, most recently from 65d75e3 to 6f9c861 Compare March 21, 2024 18:56
@salonig23 salonig23 force-pushed the erikw/feat/log-retention branch 2 times, most recently from 76f85f0 to 3524b9a Compare March 26, 2024 17:56
@salonig23 salonig23 force-pushed the erikw/feat/log-retention branch 2 times, most recently from ad0d3cc to af20a5c Compare March 26, 2024 21:37
@salonig23 salonig23 force-pushed the retain-logs-cli branch 2 times, most recently from 849c354 to 2f0bc0e Compare March 26, 2024 23:28
@salonig23 salonig23 force-pushed the retain-logs-cli branch 2 times, most recently from 53fc2e9 to e4b392b Compare March 26, 2024 23:34
@salonig23 salonig23 closed this Mar 27, 2024
@salonig23 salonig23 reopened this Mar 27, 2024
@salonig23 salonig23 changed the base branch from erikw/feat/log-retention to main March 28, 2024 00:09
Copy link

netlify bot commented Mar 28, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit c2f5e39
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/6605cce3e95c9300087561e8
😎 Deploy Preview https://deploy-preview-8982--determined-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@salonig23 salonig23 merged commit 2c7d2a1 into main Mar 28, 2024
82 of 87 checks passed
@salonig23 salonig23 deleted the retain-logs-cli branch March 28, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants