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

Move failed airlock requests to "failed" state #2395

Conversation

yuvalyaron
Copy link
Collaborator

@yuvalyaron yuvalyaron commented Aug 2, 2022

Fixes #2268
Fixes #2339

What is being addressed

Failed airlock requests no longer get stuck on "submitted" status, instead, their status changes to "failed", along with an error_message field that was added to the airlock request and describes what went wrong.

How is this addressed

  • Added a new airlock status - "Failed"
  • Added an error_message field to airlock requests to describe why the request failed.
  • Added a try catch block to StatusChangedQueueTrigger, the block wraps the main method and handles all exceptions.
  • Added event grind as an output binding to StatusChangedQueueTrigger - to publish the failed event from the function.
  • Removed the exception AirlockInvalidContainerException in airlock and added 2 exceptions that are more specific instead.

@yuvalyaron yuvalyaron changed the title Yuvalyaron feature/1967 airlock move failed requests to failed state Airlock - move failed airlock requests to "failed" state Aug 2, 2022
@github-actions
Copy link

github-actions bot commented Aug 2, 2022

Unit Test Results

494 tests   492 ✔️  12s ⏱️
    2 suites      2 💤
    2 files        0

Results for commit 552bf8f.

♻️ This comment has been updated with latest results.

@tamirkamara tamirkamara changed the title Airlock - move failed airlock requests to "failed" state Move failed airlock requests to "failed" state Aug 3, 2022
…alyaron-feature/1967-airlock-move-failed-requests-to-failed-state

� Conflicts:
�	CHANGELOG.md
…alyaron-feature/1967-airlock-move-failed-requests-to-failed-state
@yuvalyaron yuvalyaron marked this pull request as ready for review August 3, 2022 11:10
Copy link
Contributor

@anatbal anatbal left a comment

Choose a reason for hiding this comment

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

LGTM, minor comment

airlock_processor/StatusChangedQueueTrigger/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tanya-borisova tanya-borisova left a comment

Choose a reason for hiding this comment

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

LGTM, just small nits

airlock_processor/StatusChangedQueueTrigger/__init__.py Outdated Show resolved Hide resolved
airlock_processor/StatusChangedQueueTrigger/__init__.py Outdated Show resolved Hide resolved
…alyaron-feature/1967-airlock-move-failed-requests-to-failed-state

� Conflicts:
�	api_app/_version.py
@yuvalyaron
Copy link
Collaborator Author

/test

@github-actions
Copy link

github-actions bot commented Aug 4, 2022

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/2795607109 (with refid 8d45e060)

(in response to this comment from @yuvalyaron)

@yuvalyaron yuvalyaron merged commit ae8c251 into microsoft:main Aug 4, 2022
@yuvalyaron yuvalyaron deleted the yuvalyaron-feature/1967-airlock-move-failed-requests-to-failed-state branch August 4, 2022 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants