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: async DELETE experiment [DET-5804] #2741

Merged
merged 6 commits into from
Jul 21, 2021
Merged

feat: async DELETE experiment [DET-5804] #2741

merged 6 commits into from
Jul 21, 2021

Conversation

stoksc
Copy link
Contributor

@stoksc stoksc commented Jul 16, 2021

Description

This change adds the DELETING and DELETE_FAILED states to experiments and changes DELETE /api/v1/experiments/<id> to work asynchronously. Now upon a DELETE, after all validation happens, the experiment is moved to the DELETING state and the deletion takes place asynchronously. If it fails, the experiment is moved to the DELETE_FAILED state. Also on master restart, all DELETING experiments are moved to DELETE_FAILED. DELETE_FAILED can be retried manually by the user, but it not retried automatically since it would likely fail if it failed recently.

Test Plan

  • automated tests
  • run an experiment locally, delete it

Commentary (optional)

Checklist

  • 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.

@netlify
Copy link

netlify bot commented Jul 16, 2021

✔️ Deploy Preview for determined-ui ready!

🔨 Explore the source changes: d9df59d

🔍 Inspect the deploy log: https://app.netlify.com/sites/determined-ui/deploys/60f8345c206771000703945c

😎 Browse the preview: https://deploy-preview-2741--determined-ui.netlify.app/

Copy link
Member

@hamidzr hamidzr left a comment

Choose a reason for hiding this comment

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

overall it lgtm I posted a question about the experiment while deletion is happening.

master/pkg/model/experiment.go Show resolved Hide resolved
master/internal/db/postgres.go Show resolved Hide resolved
@hamidzr hamidzr assigned stoksc and unassigned hamidzr Jul 20, 2021
@stoksc stoksc assigned hamidzr and unassigned stoksc Jul 20, 2021
Copy link
Member

@hamidzr hamidzr left a comment

Choose a reason for hiding this comment

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

🚢

@stoksc stoksc merged commit 866670b into determined-ai:master Jul 21, 2021
@stoksc stoksc deleted the async-delete-experiment branch July 21, 2021 16:55
@dannysauer dannysauer modified the milestones: 0.0.102, 0.16.4 Feb 6, 2024
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.

3 participants