Handle request cancellation in Airlock processor

[yuvalyaron]

Resolves #1968

What is being addressed

• Added handling for Airlock request cancellations by deleting the container of the request.
• Refactored StatusChangedQueueTrigger

How is this addressed

• Made StatusChangedQueueTrigger send a toDeleteEvent when a request is cancelled.
• Made the API send previous_status in addition to new_status in "status changed" messages
• Added support for container deletion in ToDeleteTrigger Azure Function
• Refactored StatusChangedQueueTrigger by extracting code from method: get_source_dest_for_copy into 2 new methods: get_storage_account for getting the source and get_storage_account_destination_for_copy for getting the target

[github-actions]

Unit Test Results

524 tests  +521   522 ✔️ +520   17s ⏱️ - 2h 13m 52s
    2 suites +    1       2 💤 +    2 
    2 files   +    1       0 ❌  -     1 

Results for commit a44e3bd. ± Comparison against base commit 7fe7a89.

This pull request removes 3 and adds 524 tests. Note that renamed tests count towards both.

test_airlock ‑ test_airlock_import_flow
test_workspace_services ‑ test_create_guacamole_service_into_aad_workspace
test_workspace_services ‑ test_create_guacamole_service_into_base_workspace

tests.shared_code.test_blob_operations.TestBlobOperations ‑ test_copy_data_adds_copied_from_metadata
tests.shared_code.test_blob_operations.TestBlobOperations ‑ test_copy_data_fails_if_no_blobs_to_copy
tests.shared_code.test_blob_operations.TestBlobOperations ‑ test_copy_data_fails_if_too_many_blobs_to_copy
tests.shared_code.test_blob_operations.TestBlobOperations ‑ test_get_blob_info_from_topic_and_subject
tests.shared_code.test_blob_operations.TestBlobOperations ‑ test_get_blob_info_from_url
tests.shared_code.test_blob_operations.TestBlobOperations ‑ test_get_blob_url_should_return_blob_url
tests.shared_code.test_blob_operations.TestBlobOperations ‑ test_get_blob_url_without_blob_name_should_return_container_url
tests.test_status_change_queue_trigger.TestDataCopyProperties ‑ test_only_specific_status_are_triggering_copy
tests.test_status_change_queue_trigger.TestDataCopyProperties ‑ test_wrong_status_raises_when_getting_storage_account_properties
tests.test_status_change_queue_trigger.TestDataCopyProperties ‑ test_wrong_type_raises_when_getting_storage_account_properties
…

♻️ This comment has been updated with latest results.

[eladiw]

A. IMO toDeleteTrigger should be called ContainerDeletionTrigger
B. toDeleteEvent can be called containerDeletionEvent

[eladiw]

great job. I had one major comment about the naming but the overall refactoring looks good!

[tanya-borisova]

I can clarify why the ToDeleteTrigger was named this way: prior to its introduction, all the other triggers were named after the episode that caused the trigger to set off. With ToDelete, this "episode" is "a new item to delete has appeared", and this is why it's called this way (for brevity). I agree it's a little clumsy though!

I support renaming it but would say that adding a "container" into its name would make the name inaccurate, as currently the event contains the URL of the blob to delete and not the whole container (which I don't think was changed in this PR). Also, the form "deletion" doesn't sound consistent with the rest of the trigger names, e.g. "BlobCreatedTrigger" still has an adjective in the name.

[yuvalyaron]

while not consistent, I don't think the term 'deletion' sounds off, I think DataDeletionTrigger and BlobDeletionTrigger are good options, as it's clear what their purpose is.
Something like BlobSentForDeletionTrigger which I think is consistent, might confuse in my opinion..
what do you think? do you have any other alternatives or favorites?
@tanya-borisova @eladiw

[tanya-borisova]

I like DataDeletionTrigger the most out of those proposed!

[yuvalyaron]

@eladiw if you also agree on DataDeletionTrigger, I will change the name in the new code as part of this PR, and will open another PR for changing the name in other places as well

[eladiw]

DataDeletionTrigger sounds good. btw, are we only deleting the blob or the entire container?
I saw in one of the files an indication that it's the container...I will tag you.

[eladiw]

approved. as discussed, in the next PR we will align all the ToDelete to DataDeletion...

[yuvalyaron]

/test

[github-actions]

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/3045331636 (with refid a4724aba)

(in response to this comment from @yuvalyaron)

[yuvalyaron]

renaming will be done in PR #2592