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: Display Log retention days and Remaining log retention days in Logs Tab #9305

Merged
merged 4 commits into from May 21, 2024

Conversation

salonig23
Copy link
Contributor

@salonig23 salonig23 commented May 3, 2024

Ticket

DET-10242

Description

In the Trials Overview page added a "Log Retention Days" field.
In the logs tab, added a small tag that shows the number of days remaining to retain the logs

Test Plan

  1. Create 2 experiments, one with only one trial and another with multiple trials with different log retention days. Ex. det e create const.yaml . --config "retention_policy.log_retention_days=100"
  2. Go to the Single Trial Experiement Details Page and verify that the Log Retention Days and the Remaining log retention days on the Log tab, and its tool tip are accurate.
Screen Shot 2024-05-21 at 12 50 55 PM
  1. Click on the 3 dots in the Right corner of the Single Trial Experiement Details Page and select Retain Logs, and change the log retention days of this Experiment and verify that the Log Retention Days and Remaining log retention days update correctly (it may take up to 5 seconds)
  2. Go to the trial details page of each trial for the second experiment and make sure that the Log Retention Days and the Remaining log retention days on the Log tab are accurate.

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.

@cla-bot cla-bot bot added the cla-signed label May 3, 2024
Copy link

netlify bot commented May 3, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit e833132
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/664d08b581bf87000813e60a
😎 Deploy Preview https://deploy-preview-9305--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 force-pushed the add-log-retention-trial-overview branch 2 times, most recently from 066a505 to 8906af2 Compare May 3, 2024 19:25
Copy link

codecov bot commented May 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.02%. Comparing base (c3b3ae6) to head (e833132).
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #9305       +/-   ##
===========================================
- Coverage   48.10%   42.02%    -6.09%     
===========================================
  Files        1219      515      -704     
  Lines      157878    50557   -107321     
  Branches     2730        0     -2730     
===========================================
- Hits        75954    21247    -54707     
+ Misses      81749    29310    -52439     
+ Partials      175        0      -175     
Flag Coverage Δ
harness ?
web ?

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

Files Coverage Δ
master/internal/api_trials.go 54.69% <ø> (ø)

... and 704 files with indirect coverage changes

@salonig23 salonig23 force-pushed the add-log-retention-trial-overview branch from 8906af2 to 4d5752f Compare May 3, 2024 20:28
@salonig23 salonig23 marked this pull request as ready for review May 6, 2024 16:36
@salonig23 salonig23 requested review from a team as code owners May 6, 2024 16:36
@salonig23 salonig23 requested a review from EmilyBonar May 6, 2024 16:36
@salonig23 salonig23 force-pushed the add-log-retention-trial-overview branch from 4d5752f to 65e84a4 Compare May 6, 2024 19:14
Copy link
Contributor

@jesse-amano-hpe jesse-amano-hpe left a comment

Choose a reason for hiding this comment

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

The Go/backend side of this looks good to me -- less sure about the web UI side of things, especially Keita's comment about the Button element.

@salonig23 salonig23 force-pushed the add-log-retention-trial-overview branch from 65e84a4 to 3251967 Compare May 7, 2024 17:29
Copy link
Contributor

@keita-determined keita-determined left a comment

Choose a reason for hiding this comment

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

added some comments, but lets talk about it when we huddle

@salonig23 salonig23 force-pushed the add-log-retention-trial-overview branch 2 times, most recently from 14e0a76 to da6a026 Compare May 10, 2024 20:12
Copy link
Contributor

@keita-determined keita-determined left a comment

Choose a reason for hiding this comment

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

mostly looks good

@salonig23 salonig23 force-pushed the add-log-retention-trial-overview branch from 88d077f to 627d2dd Compare May 21, 2024 17:15
Copy link
Contributor

@keita-determined keita-determined left a comment

Choose a reason for hiding this comment

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

could you fix the PR title? its truncated.
CI is failing
other than that lgtm

webui/react/src/pages/TrialDetails/TrialInfoBox.test.tsx Outdated Show resolved Hide resolved
@salonig23 salonig23 changed the title feat: Display Log retention days and Remaining log retention days in … feat: Display Log retention days and Remaining log retention days in Logs Tab May 21, 2024
@salonig23 salonig23 force-pushed the add-log-retention-trial-overview branch from 627d2dd to e833132 Compare May 21, 2024 20:48
@salonig23 salonig23 merged commit 3bbb51a into main May 21, 2024
81 of 96 checks passed
@salonig23 salonig23 deleted the add-log-retention-trial-overview branch May 21, 2024 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants