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: Flat Run Actions #9368

Merged
merged 33 commits into from
Jun 27, 2024
Merged

feat: Flat Run Actions #9368

merged 33 commits into from
Jun 27, 2024

Conversation

keita-determined
Copy link
Contributor

@keita-determined keita-determined commented May 14, 2024

Ticket

ET-110

Description

Flat Run Actions
Tensorboard and Pause/Unpause actions are out of this PR, so not included
Move action will be implemented after #9390 is merged into this

Test Plan

  • Enable Run table
  • Check if action button is visible when one or more flat runs are selected
  • Check if archive, unarchive, delete, kill actions are working with/without RBAC
  • If runs belong to searches, the warning modal should show up. if user cancel it, move action won't happen.

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 14, 2024
@keita-determined keita-determined changed the title feat: initial implementation of flat run actions feat: Flat run actions May 14, 2024
Copy link

netlify bot commented May 14, 2024

Deploy Preview for determined-ui canceled.

Name Link
🔨 Latest commit 61d5dd5
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/667dd973765e060008d18483

@keita-determined keita-determined changed the title feat: Flat run actions feat: Flat Run Actions May 14, 2024
Copy link

codecov bot commented May 14, 2024

Codecov Report

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

Project coverage is 43.29%. Comparing base (ca45198) to head (faa104d).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9368      +/-   ##
==========================================
- Coverage   48.57%   43.29%   -5.28%     
==========================================
  Files        1234      911     -323     
  Lines      158841   119262   -39579     
  Branches     2778     2779       +1     
==========================================
- Hits        77155    51634   -25521     
+ Misses      81511    67452   -14059     
- Partials      175      176       +1     
Flag Coverage Δ
harness ?
web 44.18% <12.53%> (-0.17%) ⬇️

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

Files Coverage Δ
webui/react/src/services/apiConfig.ts 73.37% <100.00%> (+0.46%) ⬆️
webui/react/src/services/decoder.ts 20.44% <25.00%> (ø)
webui/react/src/services/api.ts 0.00% <0.00%> (ø)
webui/react/src/pages/FlatRuns/FlatRuns.tsx 0.00% <0.00%> (ø)
...i/react/src/pages/FlatRuns/FlatRunActionButton.tsx 0.00% <0.00%> (ø)

... and 324 files with indirect coverage changes

@keita-determined keita-determined force-pushed the feature/actions-flat-run-table branch 2 times, most recently from faa104d to 7eee64d Compare May 23, 2024 23:32
@codecov-commenter
Copy link

codecov-commenter commented May 23, 2024

Codecov Report

Attention: Patch coverage is 83.62919% with 166 lines in your changes missing coverage. Please review.

Project coverage is 51.27%. Comparing base (da9025b) to head (61d5dd5).
Report is 19 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9368      +/-   ##
==========================================
+ Coverage   49.79%   51.27%   +1.47%     
==========================================
  Files        1247     1252       +5     
  Lines      162235   151993   -10242     
  Branches     2888     3018     +130     
==========================================
- Hits        80793    77929    -2864     
+ Misses      81270    73905    -7365     
+ Partials      172      159      -13     
Flag Coverage Δ
backend 43.88% <100.00%> (+<0.01%) ⬆️
harness 72.80% <ø> (+9.05%) ⬆️
web 47.85% <83.51%> (+1.68%) ⬆️

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

Files Coverage Δ
master/internal/api_runs.go 63.62% <100.00%> (+0.38%) ⬆️
...eact/src/components/InterstitialModalComponent.tsx 100.00% <100.00%> (+100.00%) ⬆️
webui/react/src/constants/states.ts 99.45% <100.00%> (ø)
webui/react/src/pages/SearchDetails.tsx 75.87% <100.00%> (+75.87%) ⬆️
.../react/src/pages/TrialDetails/TrialDetailsLogs.tsx 15.60% <100.00%> (+0.47%) ⬆️
webui/react/src/services/api.ts 97.64% <100.00%> (+0.07%) ⬆️
webui/react/src/services/decoder.ts 21.67% <100.00%> (+1.54%) ⬆️
webui/react/src/types.ts 99.68% <100.00%> (+<0.01%) ⬆️
webui/react/src/utils/mergeAbortControllers.ts 100.00% <100.00%> (ø)
webui/react/src/utils/tests/generateTestData.ts 91.11% <100.00%> (+0.02%) ⬆️
... and 10 more

... and 13 files with indirect coverage changes

@keita-determined
Copy link
Contributor Author

API doesnt not include archived value in FlatRun, so expose it if its not intentional

@keita-determined keita-determined force-pushed the feature/actions-flat-run-table branch 5 times, most recently from 20a458a to f624a38 Compare May 28, 2024 19:18
@keita-determined keita-determined marked this pull request as ready for review May 28, 2024 19:19
@keita-determined keita-determined requested review from a team as code owners May 28, 2024 19:19
@keita-determined keita-determined requested review from gt2345, hamidzr, ashtonG, corban-beaird and AmanuelAaron and removed request for hamidzr and gt2345 May 28, 2024 19:19
@keita-determined keita-determined force-pushed the feature/actions-flat-run-table branch from f624a38 to 763becb Compare May 28, 2024 22:44
@keita-determined keita-determined force-pushed the feature/actions-flat-run-table branch from 763becb to 6919563 Compare May 30, 2024 19:12
@keita-determined keita-determined force-pushed the feature/actions-flat-run-table branch 2 times, most recently from e87ef44 to db64742 Compare June 3, 2024 18:07
@keita-determined keita-determined force-pushed the feature/actions-flat-run-table branch from 0fe119f to f61cde3 Compare June 24, 2024 20:40
@keita-determined
Copy link
Contributor Author

talked to design -- let's use the pencil icon instead of the fire icon for the action menu, please!

fixed

Some runs you are trying to move belong to a Hyperparameter Search and cannot be moved
independently without breaking their contextual relationships. These runs will be moved with
their parent search.
{`Some of the runs you're trying to move are part of a hyperparameter search. To preserve their
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to avoid "' can be escaped with &apos;, &lsquo;, &#39;, &rsquo;." error, used string here

@@ -729,6 +732,11 @@ func archiveUnarchiveAction(ctx context.Context, archive bool, runIDs []int32,
for _, cand := range runCandidates {
visibleIDs.Insert(cand.ID)
switch {
case cand.ExpArchived:
results = append(results, &apiv1.RunActionResult{
Error: "Run is part of archived Search.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we be able to include what search it's part of?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed it

Copy link
Contributor

@corban-beaird corban-beaird left a comment

Choose a reason for hiding this comment

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

BE side of things looks good, I had one nit about an error message

@keita-determined keita-determined merged commit bd633f1 into main Jun 27, 2024
82 of 97 checks passed
@keita-determined keita-determined deleted the feature/actions-flat-run-table branch June 27, 2024 21:52
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.

6 participants