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

Fix process load test results workflow #6869

Merged
merged 4 commits into from
Aug 14, 2024
Merged

Fix process load test results workflow #6869

merged 4 commits into from
Aug 14, 2024

Conversation

TharmiganK
Copy link
Contributor

@TharmiganK TharmiganK commented Aug 14, 2024

Purpose

The load-tests has been failing for some time due to the following issue:

  • The merge PR step hangs intermittently and get timed-out, which makes the PR not merged until the next load test results PR
  • Due to the above reason, there is already a load-test-results branch which is not sync with the master, which makes the push branch step fail

In this PR, the above issues has been addressed with the following changes:

  • Use a github CLI command to wait until all the required checks pass
  • Check for already existing branch and remove it before pushing the new changes - Ideally this check is not required when the PR is merged automatically

Copy link
Contributor

@NipunaRanasinghe NipunaRanasinghe left a comment

Choose a reason for hiding this comment

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

LGTM

@ThisaruGuruge
Copy link
Member

Check for already existing branch and remove it before pushing the new changes - Ideally this check is not required when the PR is merged automatically

I am -1 for this. We have discussed this previously as well. If a load test PR was failed due to a GitHub issue, we will lose that load test result because of this. The recommended way is to module owners to manually handle this (or maybe we can come up with a way to automate this, but I'm not sure).

Manually handling:

  1. Close the failed PR
  2. Delete the branch
  3. Re-run the failed load test processing workflows

@TharmiganK
Copy link
Contributor Author

I am -1 for this. We have discussed this previously as well. If a load test PR was failed due to a GitHub issue, we will lose that load test result because of this. The recommended way is to module owners to manually handle this (or maybe we can come up with a way to automate this, but I'm not sure).

Ack, will remove the deleting part.

May be we can send a notification to a notification chat group with the library team members, so that they can address it asap

@TharmiganK TharmiganK marked this pull request as draft August 14, 2024 09:58
Copy link
Member

@ThisaruGuruge ThisaruGuruge left a comment

Choose a reason for hiding this comment

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

LGTM

+1 for sending a message.

@TharmiganK TharmiganK marked this pull request as ready for review August 14, 2024 13:51
@TharmiganK
Copy link
Contributor Author

Since the workflows are failing currently in the merge PR step, I will merge this change and test. Sending notification on failure will be addressed separately

@TharmiganK TharmiganK merged commit 927dfcb into main Aug 14, 2024
2 checks passed
@ThisaruGuruge ThisaruGuruge deleted the load-test-fix branch August 29, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants