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

Skip sending step result events for Airlock updates that do not include a status change #2547

Merged
merged 9 commits into from
Sep 7, 2022

Conversation

yuvalyaron
Copy link
Collaborator

@yuvalyaron yuvalyaron commented Sep 5, 2022

Resolves #2540, #2534

What is being addressed

As part of #2504, Airlock requests might be updated without updating their status (for instance, enumerating the request files adds the files to the request without changing its status), however the method that updates the requests is also responsible for sending status changed events, so when there is an update to an Airlock request, a status changed event is triggered, in cases where it was not necessary, such as in file enumeration this resulted in duplicated step results.

How is this addressed

  • If there is no status change in update_and_publish_event_airlock_request, we skip the send of status changed event
  • Added unit tests to enforce the previous bullet
  • Changed the E2E test to stop polling the request status if the current status is final and will not change.

@github-actions
Copy link

github-actions bot commented Sep 5, 2022

Unit Test Results

499 tests   497 ✔️  12s ⏱️
    1 suites      2 💤
    1 files        0

Results for commit a3ab664.

♻️ This comment has been updated with latest results.

@yuvalyaron yuvalyaron changed the title Skip sending step result events for updates without new status Skip sending step result events for Airlock updates that do not include a status change Sep 5, 2022
@yuvalyaron
Copy link
Collaborator Author

/help

@github-actions
Copy link

github-actions bot commented Sep 5, 2022

🤖 pr-bot 🤖

Hello!

You can use the following commands:
    /test - build, deploy and run smoke tests on a PR
    /test-extended - build, deploy and run smoke & extended tests on a PR
    /test-extended-aad - build, deploy and run smoke & extended AAD tests on a PR
    /test-shared-services - test the deployment of shared services on a PR build
    /test-force-approve - force approval of the PR tests (i.e. skip the deployment checks)
    /test-destroy-env - delete the validation environment for a PR (e.g. to enable testing a deployment from a clean start after previous tests)
    /help - show this help

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

@github-actions
Copy link

github-actions bot commented Sep 5, 2022

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/2994413365 (with refid 63211578)

(in response to this comment from @yuvalyaron)

@github-actions
Copy link

github-actions bot commented Sep 5, 2022

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/2994413365 (with refid 63211578)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-destroy-env

@github-actions
Copy link

github-actions bot commented Sep 5, 2022

Destroying PR test environment (RG: rg-tre63211578)... (run: https://github.com/microsoft/AzureTRE/actions/runs/2996135545)

@yuvalyaron yuvalyaron added bug Something isn't working airlock labels Sep 5, 2022
@github-actions
Copy link

github-actions bot commented Sep 5, 2022

PR test environment destroy complete (RG: rg-tre63211578)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/2997935124 (with refid 63211578)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-destroy-env

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

Destroying PR test environment (RG: rg-tre63211578)... (run: https://github.com/microsoft/AzureTRE/actions/runs/2999449040)

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

PR test environment destroy complete (RG: rg-tre63211578)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/2999726926 (with refid 63211578)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-destroy-env

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

Destroying PR test environment (RG: rg-tre63211578)... (run: https://github.com/microsoft/AzureTRE/actions/runs/3001187006)

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

PR test environment destroy complete (RG: rg-tre63211578)

@yuvalyaron yuvalyaron marked this pull request as ready for review September 6, 2022 16:13
@yuvalyaron
Copy link
Collaborator Author

/test-extended

@github-actions
Copy link

github-actions bot commented Sep 6, 2022

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/3001737037 (with refid 63211578)

(in response to this comment from @yuvalyaron)

@yuvalyaron
Copy link
Collaborator Author

/test-extended

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

🤖 pr-bot 🤖

🏃 Running extended tests: https://github.com/microsoft/AzureTRE/actions/runs/3005766060 (with refid 63211578)

(in response to this comment from @yuvalyaron)

@tamirkamara tamirkamara requested a review from eladiw September 7, 2022 10:45
Copy link
Contributor

@eladiw eladiw 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 suggestion

Co-authored-by: Elad Iwanir <13205761+eladiw@users.noreply.github.com>
@yuvalyaron
Copy link
Collaborator Author

/test

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

🤖 pr-bot 🤖

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

(in response to this comment from @yuvalyaron)

@yuvalyaron yuvalyaron enabled auto-merge (squash) September 7, 2022 11:33
@yuvalyaron
Copy link
Collaborator Author

/test

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

🤖 pr-bot 🤖

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

(in response to this comment from @yuvalyaron)

@yuvalyaron yuvalyaron merged commit 0bada6e into microsoft:main Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
airlock bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Airlock request fails in E2E
2 participants