Add further creation waits to notebooks tests #1270
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Simeon Widdis <[email protected]>
Swiddis
requested review from
tianleh,
kavilla,
ohltyler,
CCongWang,
ashwin-pc,
peterzhuamazon,
ananzh,
prudhvigodithi,
xluo-aws,
Hailong-am,
SuZhou-Joe,
ruanyl,
wanglam and
raintygao
as code owners
Signed-off-by: Simeon Widdis <[email protected]>
Signed-off-by: Simeon Widdis <[email protected]>
|
After discussing with the team, we decided to keep the refresh/reload logic present while still checking for creation, to try and fix the flakiness. I've added an issue link in-code to ensure this is removed when the underlying bug is fixed. |
Swiddis
changed the title
|
|
This reverts commit 6dca5a7. Signed-off-by: Simeon Widdis <[email protected]>
joshuali925
approved these changes
May 6, 2024
ashwin-pc
approved these changes
May 6, 2024
Hailong-am
approved these changes
May 6, 2024
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
May 7, 2024
* Replace refresh with wait for creation Signed-off-by: Simeon Widdis <[email protected]> * Remove unused variable Signed-off-by: Simeon Widdis <[email protected]> * Re-add refresh/reload with issue ref Signed-off-by: Simeon Widdis <[email protected]> * Revert "Re-add refresh/reload with issue ref" This reverts commit 6dca5a7. Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Simeon Widdis <[email protected]> (cherry picked from commit 8014ae0)
opensearch-trigger-bot bot
pushed a commit
that referenced
this pull request
May 7, 2024
* Replace refresh with wait for creation Signed-off-by: Simeon Widdis <[email protected]> * Remove unused variable Signed-off-by: Simeon Widdis <[email protected]> * Re-add refresh/reload with issue ref Signed-off-by: Simeon Widdis <[email protected]> * Revert "Re-add refresh/reload with issue ref" This reverts commit 6dca5a7. Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Simeon Widdis <[email protected]> (cherry picked from commit 8014ae0)
peterzhuamazon
pushed a commit
that referenced
this pull request
May 7, 2024
* Replace refresh with wait for creation Signed-off-by: Simeon Widdis <[email protected]> * Remove unused variable Signed-off-by: Simeon Widdis <[email protected]> * Re-add refresh/reload with issue ref Signed-off-by: Simeon Widdis <[email protected]> * Revert "Re-add refresh/reload with issue ref" This reverts commit 6dca5a7. Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Simeon Widdis <[email protected]> (cherry picked from commit 8014ae0) Co-authored-by: Simeon Widdis <[email protected]>
SuZhou-Joe
pushed a commit
that referenced
this pull request
May 7, 2024
* Replace refresh with wait for creation Signed-off-by: Simeon Widdis <[email protected]> * Remove unused variable Signed-off-by: Simeon Widdis <[email protected]> * Re-add refresh/reload with issue ref Signed-off-by: Simeon Widdis <[email protected]> * Revert "Re-add refresh/reload with issue ref" This reverts commit 6dca5a7. Signed-off-by: Simeon Widdis <[email protected]> --------- Signed-off-by: Simeon Widdis <[email protected]> (cherry picked from commit 8014ae0) Co-authored-by: Simeon Widdis <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Since notebooks are still flaky on notebook creation, even with refresh logic added (see #1250), I think the reason the tests are still failing is because notebook creation is flaky in some index refresh race condition scenarios. This PR refactors the notebook creation to drop the refresh attempt and only check for a successful creation message, on the assumption that notebooks should have a functional requirement to always show a valid notebook after stating a successful creation.
I'm not sure this fixes the flaky tests in CI (still struggling to understand the root cause and get exact repro steps), but in local testing this passed 10 consecutive runs (the old version failed after 6 runs and a version with both parts fails almost every run). This should also pass the ball out of the testing repo's court, I've created a corresponding issue: opensearch-project/dashboards-observability#1822.
Also needs backport to 2.x and main.
Issues Resolved
N/A
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.