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.

Sign up for GitHub

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

Jump to bottom

fix(snapshot): Cleanup notification handling for snapshots #917

Merged

andrewazores merged 14 commits into cryostatio:main from hareetd:fix-snapshot-creation-notifications

Apr 29, 2022

Contributor

hareetd commented Apr 29, 2022 •

edited by andrewazores

Fixes #916
Depends on cryostatio/cryostat-web#427

Hareet Dhillon added 5 commits

April 27, 2022 19:09


          Issue snapshot creation notification once the snapshot has passed ver…

17e1dd7

…ification


          fixup! Issue snapshot creation notification once the snapshot has pas…

4d3ba5c

…sed verification


          fixup! Issue snapshot creation notification once the snapshot has pas…

8a2ba2e

…sed verification


          Add notifications for snapshot creation and deletion

37032ed


          Add regex to determine whether a recording up for deletion is a snaps…

5b3e9d1

…hot or not)

hareetd added cleanup chore labels

hareetd requested a review from andrewazores

April 29, 2022 18:17

Contributor Author

hareetd commented Apr 29, 2022

Thoughts before I finish updating tests?


          Run spotless

a8be83c

andrewazores reviewed

View reviewed changes

src/main/java/io/cryostat/net/web/http/api/v1/TargetRecordingDeleteHandler.java Outdated Show resolved Hide resolved

andrewazores reviewed

View reviewed changes

Member

andrewazores left a comment

Looking good so far, just that one refactoring idea I commented

Hareet Dhillon added 2 commits

April 29, 2022 14:56


          Extract determining whether a recording is a snapshot or not into the…

17eb623

… RecordingTargetHelper


          fixup! Extract determining whether a recording is a snapshot or not i…

3dcab10

…nto the RecordingTargetHelper

andrewazores reviewed

View reviewed changes

src/main/java/io/cryostat/recordings/RecordingTargetHelper.java Outdated Show resolved Hide resolved

andrewazores reviewed

View reviewed changes

src/main/java/io/cryostat/recordings/RecordingTargetHelper.java Outdated Show resolved Hide resolved

Hareet Dhillon added 2 commits

April 29, 2022 15:14


          Simplify notification handling for deleted recordings

ba3c4c8


          fixup! Simplify notification handling for deleted recordings

f00f4ae

andrewazores reviewed

View reviewed changes

Member

andrewazores left a comment

Implementation looks good.

Hareet Dhillon added 3 commits

April 29, 2022 16:56


          Finish updating tests

0151e16


          Run spotless

9f3f3ab


          Add more snapshot name test strings

35d1d93

andrewazores previously approved these changes

View reviewed changes

andrewazores changed the title ~~chore(snapshot): Cleanup notification handling for snapshots~~ fix(snapshot): Cleanup notification handling for snapshots

andrewazores added fix and removed cleanup chore labels

andrewazores dismissed their stale review

April 29, 2022 21:48

Dismissing for now - change looks good but should include corresponding web-client PR once that is merged

github-actions bot added the dependent label


          Bump web client commit

c7e6eff

github-actions bot removed the dependent label

Contributor

github-actions bot commented Apr 29, 2022

This PR/issue depends on:

~~fix(snapshots): remove successful snapshot creation notification handling cryostat-web#427~~
By Dependent Issues (🤖). Happy coding!

andrewazores approved these changes

View reviewed changes

andrewazores merged commit 75832b5 into cryostatio:main

hareetd deleted the fix-snapshot-creation-notifications branch

April 29, 2022 22:41

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment