Skip to content
Snippets Groups Projects

Ensure deletion of Project, Resource, and FileObject

Closed Niklas Siemer requested to merge ensure_delete into master
1 unresolved thread

Currently, the delete method on the objects is to quickly called in my opinion. Since, there is no trash bin a single false call could result in a lot of harm. Therefore, I propose to ensure the method a bit by requiring an additional enable parameter to be set to True before giving the call to the Coscine API.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Hmm, i don't know about this. Shouldn't a call to delete() always imply that a user would like to delete something? Otherwise it would be quite useless. Can you think of an actual use case where an accidental delete could happen (apart from the first initial run of a new script)?
    Maybe instead of that parameter we should instead introduce a global (i.e. applying to all or a subset of functions) parameter, that puts the client in simulation mode or something similar to that, where each operation is logged but not executed (at least not remote on Coscine servers, instead in a local environment replicated with folders for dummy projects). This could help a user with testing some functionality they implemented, without actually having to create/delete projects for example. However that also sounds a lot harder to implement...
    Or maybe a global "interactive" mode parameter, that for critical stuff prompts the user, whether they would like to delete something. That on the other hand would not work for GUIs since the prompt would only appear in the cli. And for 1000 files it would be quite the hassle.

    • Actually, the case where one writes a new script is my biggest concern. One could of course couple this with an interactive mode (asking for approval) like in

      def delete(self, interactive=None):
         interactive = interactive or global_interactive
         
         if interactive:
            accepted = input('Are you sure to delete? Y/n')
         else:
            accepted = 'y'
      
         if accepted.lower() in ['y', 'yes']:
            client.delete(...)

      with such a pattern one could use a global interactive lock and get rid of it in loops... However, I think this could be overdoing it somehow xD

      One thing I actually would like is a read_only mode, which could be defined on the client itself. If Client.read_only == True I would only allow for Client.get operations.

    • I see now, that definitely makes a lot of sense.

    • I would say, that a global setting like that read_only/safe mode is the way to go then, or would it make sense to keep deletion enabled for i. e. projects and resources but not for file-objects? Otherwise one may forget about the argument at some point in the sourcecode (when multiple delete statements are used in different files) and then does not enable it for the production/live version.
      A read-only mode that is enabled by default and must be explicitly switched off at one point in the sourcecode would mitigate that issue in my opinion. Adding a global interactive-mode etc. would also be easier in the future, rather than Something.delete(enable=True, interactive=False). In that case one could of course also do

      client.read_only = True
      precious_project.delete()
      client.read_only = False

      but generally my approach would look like this:

      import coscine
      client = coscine.Client(token, some_settings, read_only = False) # need to explicitly enable writing
      client.delete_everything_and_cause_havoc()
    • Please register or sign in to reply
  • I have added a read_only setting for v0.8.0, which only allows GET requests. Other requests are silently disregarded and when using the logging module with at least level warn, a warning is printed to the cli. I have not thoroughly tested this mode and there may be some issues when one of the disregarded requests does not take place, but during my limited testing I did not stumble upon any errors. Maybe this is already good enough for testing client code.

    If users want to be more careful around delete/post stuff, they would have to include an if statement with an interactive prompt or something like that before using any potentially harmful code like project.delete() in their production code.

    Edited by Romin
  • closed

Please register or sign in to reply
Loading