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

[DC-1170] Add code for get and delete requests #1747

Merged
merged 10 commits into from
Jul 26, 2024

Conversation

s-rubenstein
Copy link
Contributor

Jira ticket: https://broadworkbench.atlassian.net/browse/dc-1170

Addresses

  1. Adds GET and DELETE for snapshot access request to help make snapshot access requests more usable.
  2. Adds a step to the Snapshot delete flight to check for outstanding requests and delete them, so that requests don't block deletion of the Sam Snapshot resource.

Summary of changes

Testing Strategy

@s-rubenstein s-rubenstein requested review from a team as code owners July 25, 2024 19:10
@s-rubenstein s-rubenstein requested review from rushtong and aarohinadkarni and removed request for a team July 25, 2024 19:10
Copy link
Member

@pshapiro4broad pshapiro4broad left a comment

Choose a reason for hiding this comment

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

Looks good, a few comments


@Override
public StepResult undoStep(FlightContext context) throws InterruptedException {
// can't undo delete
Copy link
Member

Choose a reason for hiding this comment

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

Is there a need to log here? I don't think anyone will look at this log message, and if they did, I don't know what they'd do with this information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need to, but I was following the example here: https://github.com/DataBiosphere/jade-data-repo/blob/develop/src/main/java/bio/terra/service/snapshot/flight/delete/DeleteSnapshotAuthzResource.java#L39

I think that might be useful information in the case that things end up in a bad state? What are your thoughts?

src/main/resources/api/data-repository-openapi.yaml Outdated Show resolved Hide resolved
Copy link

@s-rubenstein s-rubenstein merged commit ce9f1a7 into develop Jul 26, 2024
13 checks passed
@s-rubenstein s-rubenstein deleted the sr/dc-1170-delete-get-endpoints-and-flight branch July 26, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants