covalent icon indicating copy to clipboard operation
covalent copied to clipboard

Web app - Deletion of dispatches

Open mshkanth opened this issue 2 years ago • 7 comments

Description This is to document reg how the 'dispatch deletion' feature is to be implemented on the web app GUI

Note - This is an existing feature in the OS app and not a new feature. This is being re-factored in the context of the DB revamp activity

Requirement

  1. A user has the option to select one or more dispatches & delete them from the web app GUI.
  2. These dispatches are then deleted in the database so as to not show those records to users.
  3. Dispatches across statuses can be deleted (~~Pending clarification from AQ backend team. Potentially, we might only have to delete completed records as the Covalent dispatcher will try to update the DB for running lattices~~)

Proposed implementation

  1. ~~Web app to remove records & files in the file system associated with the dispatch.~~ Web app to soft delete records from the database. Web app to leave any files un-touched in the spirit of a soft delete.
  2. ~~Records will be removed from all the 3 tables - Lattices, electrons & electron_dependency.~~ Web app to only soft delete using the is_active field
  3. ~~The corresponding folder/files under lattices.storage_path will be deleted.~~ Web app to leave any files un-touched in the spirit of a soft delete.
  4. In the event of multiple dispatches being deleted, an 'All or none' approach will be taken to keep error handling simple.

Future enhancements

  1. A partial success case can be implemented in the event that an error occurs on deletion of a specific dispatch.
  2. Implementation can be extended to additionally 'stop' or 'cancel' dispatches that are already running. This will likely involve an integration between Web app & Covalent dispatcher.

mshkanth avatar Jul 12 '22 14:07 mshkanth

Hey team! Please add your planning poker estimate with ZenHub @ArunPsiog @Prasy12

mshkanth avatar Jul 12 '22 14:07 mshkanth

@wjcunningham7 @FyzHsn - Giving you visibility reg 'Delete dispatch' feature on the Web app GUI per requirement from Santosh. Pls comment if you have any thoughts or tag anyone else.

Cc @santoshkumarradha

mshkanth avatar Jul 12 '22 14:07 mshkanth

Hi @FyzHsn - We are thinking of either of the following implementations reg 'Deletion of dispatches from the web app GUI' -

  1. Permit a user to only delete completed dispatches (Status of completed, failed and cancelled)
  2. Permit a user to delete inprogress & completed dispatches (Status of new_object, running, completed, failed and cancelled)

I am guessing your preference would be Option 1 since the covalent dispatcher will try to update dispatch records in the DB and it would mess things up if we deleted them. Let me know what you think?

Cc @wjcunningham7

mshkanth avatar Jul 13 '22 12:07 mshkanth

@mshkanth @mshkanth My preference would be to add a field deleted field in the tables to indicate that something has been deleted in the UI by the user. At this moment, my advice is to not mess with deleting actual data. If anyone needs to filter out the deleted workflows, they can simply use this field.

This would be true for both completed and in-progress workflows. Also, the logic of what to do with in-progress workflows if they get deleted is not clear to me. For example, when a user deletes a running workflow, perhaps the protocol is to cancel it first and then change the deleted status to True.

FyzHsn avatar Jul 13 '22 13:07 FyzHsn

@FyzHsn - I agree that doing a soft delete is the way to go. Can you add a deleted or is_active field to all the 3 tables? I'l also add this to the database schema discussion.

Reg deleting inprogress flows, Yes, the idea flow would be to issue a cancel request (presumably to the /api/cancel API spec'd out inside covalent_dispatcher/_service/app.py) and after confirmation, soft delete those records in the database. We will park this for now and take this up as an improvement after we implement the DB schema revamp on our side.

Cc @Prasy12 @mpvgithub @ArunPsiog @Aravind-psiog

mshkanth avatar Jul 13 '22 14:07 mshkanth

Conclusion based on offline discussion between Team Psi, @wjcunningham7 & @FyzHsn -

  • Team AQ to introduce a is_active flag in all 3 tables as part of the next DB version release.
  • This field will be updated in the short term by the Web app whenever a user 'deletes' a dispatch. Along with this, the updated_at will be updated for all such updates.
  • Web app to leave any files un-touched in the spirit of a soft delete. Only DB records will be marked with a soft delete.
  • For now, 'deletion' is to be intrepreted as 'hiding from the GUI. In the medium term, stopping/cancelling dispatches can be taken up.
  • The GUI will permit deletion across dispatch statuses.

I will update the issue description to stay up to date with the final conclusion.

mshkanth avatar Jul 18 '22 12:07 mshkanth

@Prasy12 @mpvgithub @ArunPsiog @Aravind-psiog - Pls check above comment & modified description

mshkanth avatar Jul 18 '22 12:07 mshkanth