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: bulk actions matching filters (ET-241) #9895

Merged
merged 8 commits into from
Oct 14, 2024

Conversation

jesse-amano-hpe
Copy link
Contributor

@jesse-amano-hpe jesse-amano-hpe commented Sep 5, 2024

Ticket

ET-241

Description

Defines new endpoints for internal use, allowing the user to move/kill/pause/etc. searches using modern filter schema. The intention is to support a "true select-all" flow in ET-238 where the user sets up some filters, and then submits a bulk action request for all searches (multi-trial experiments) matching that filter.

Test Plan

Checklist

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

@cla-bot cla-bot bot added the cla-signed label Sep 5, 2024
Copy link

netlify bot commented Sep 5, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit 06427ec
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/670d93f67e784a0008c556db
😎 Deploy Preview https://deploy-preview-9895--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.

@jesse-amano-hpe jesse-amano-hpe force-pushed the jta/select-all-matching-filters-ET-241 branch 2 times, most recently from 6528b04 to 72185dd Compare September 10, 2024 06:49
@jesse-amano-hpe jesse-amano-hpe marked this pull request as ready for review September 10, 2024 06:50
@jesse-amano-hpe jesse-amano-hpe requested a review from a team as a code owner September 10, 2024 06:50
@jesse-amano-hpe jesse-amano-hpe requested review from ShreyaLnuHpe and AmanuelAaron and removed request for ShreyaLnuHpe September 10, 2024 06:50
IsTerminal bool
}

func filterSearchQuery(getQ *bun.SelectQuery, filter *string) (*bun.SelectQuery, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any difference between this function and the one in api_runs.go? If not maybe we can import it, or move it to a util file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The only difference is that since we're not querying the runs table to filter searches, we only look at WHERE NOT e.archived. Kind of a tricky thing to factor out but I'm open to thinking more on how we might consolidate this.

Join("JOIN workspaces w ON p.workspace_id = w.id")
}

func (a *apiServer) MoveSearches(
Copy link
Contributor

Choose a reason for hiding this comment

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

Add tests for these new functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Sorry it took so long!

@jesse-amano-hpe
Copy link
Contributor Author

Note from chatting with Alex today -- I'm going to add the tests, but we're going to let this sit unmerged until after the release cut next week (as it happens, I'm going to do the cutting) so we can have this endpoint included in the same wave of release party testing as the frontend feature it goes with instead of potentially doubling+ the QA work.

@jesse-amano-hpe jesse-amano-hpe force-pushed the jta/select-all-matching-filters-ET-241 branch from 72185dd to b975a5f Compare October 1, 2024 12:40
Copy link

codecov bot commented Oct 1, 2024

Codecov Report

Attention: Patch coverage is 54.88889% with 203 lines in your changes missing coverage. Please review.

Project coverage is 54.41%. Comparing base (ac82b3c) to head (06427ec).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
master/internal/api_searches.go 54.88% 203 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #9895    +/-   ##
========================================
  Coverage   54.40%   54.41%            
========================================
  Files        1261     1262     +1     
  Lines      158430   158880   +450     
  Branches     3631     3631            
========================================
+ Hits        86201    86448   +247     
- Misses      72095    72298   +203     
  Partials      134      134            
Flag Coverage Δ
backend 45.46% <54.88%> (+0.07%) ⬆️
harness 72.74% <ø> (ø)
web 53.95% <ø> (ø)

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

Files with missing lines Coverage Δ
master/internal/api_searches.go 54.88% <54.88%> (ø)

Copy link
Contributor

@AmanuelAaron AmanuelAaron left a comment

Choose a reason for hiding this comment

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

LGTM

@jesse-amano-hpe jesse-amano-hpe force-pushed the jta/select-all-matching-filters-ET-241 branch from ef57707 to a94569d Compare October 2, 2024 19:08
@jesse-amano-hpe jesse-amano-hpe force-pushed the jta/select-all-matching-filters-ET-241 branch from a94569d to 3a5c3af Compare October 14, 2024 21:02
@jesse-amano-hpe jesse-amano-hpe merged commit 2ef2f12 into main Oct 14, 2024
81 of 94 checks passed
@jesse-amano-hpe jesse-amano-hpe deleted the jta/select-all-matching-filters-ET-241 branch October 14, 2024 22:45
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.

2 participants