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

Update github.actor with github.event.pull_request.user.login for dependabot[bot] CI's #13688

Merged
merged 1 commit into from
May 15, 2024

Conversation

prudhvigodithi
Copy link
Member

@prudhvigodithi prudhvigodithi commented May 15, 2024

Description

Coming from https://github.com/orgs/community/discussions/25502 and the issue pointed by @cwperks that Pull Request Checks is being run on this PR: #13643 raised by dependabot[bot].

But actually for the above PR looks to me like the trigger is coming from different bot https://github.com/opensearch-project/OpenSearch/actions/runs/9063816615 which is opensearch-trigger-bot[bot].

Adding github.event.pull_request.user.login to get the author who opened this PR and exclude the check for dependabot[bot].

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • API changes companion pull request created.
  • Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)
  • Public documentation issue/PR created

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.

@prudhvigodithi prudhvigodithi requested a review from cwperks May 15, 2024 17:57
@cwperks
Copy link
Member

cwperks commented May 15, 2024

Thank you @prudhvigodithi!

Its interesting, because I tested this on my fork and the GHA runs as dependabot (github.actor = dependabot[bot]). Example: https://github.com/cwperks/OpenSearch/actions/runs/9056524038?pr=163

In the main repo, the GHA is not run as dependabot for dependabot PRs. Its running as opensearch-trigger-bot instead. Example: https://github.com/opensearch-project/OpenSearch/actions/runs/9063816615?pr=13643

This change looks good to me! Approved. (Verbally approving because my approval counts towards merge, but I'm not a maintainer of this repo)

@prudhvigodithi
Copy link
Member Author

Thanks @cwperks here is the quick test run that prints the PR author with github.event.pull_request.user.login and skips the run based on github.event.pull_request.user.login, please check
https://github.com/prudhvigodithi/opensearch-build/actions/runs/9100626642/job/25015897504?pr=104

@cwperks
Copy link
Member

cwperks commented May 15, 2024

Run echo "This pull request was created by prudhvigodithi"
This pull request was created by prudhvigodithi

Thank you @prudhvigodithi. The output is what I expect, the github username of the submitter of the pull request.

Can you also update this instance as well?

@cwperks
Copy link
Member

cwperks commented May 15, 2024

Both of these actions are only run on PRs, so I expect github.event.pull_request.user.login will always be populated, but for actions that run on push would this value be empty?

@prudhvigodithi prudhvigodithi changed the title Update Pull Request Checks to exclude dependabot[bot] Update github.actor with github.event.pull_request.user.login for dependabot[bot] CI's May 15, 2024
@prudhvigodithi
Copy link
Member Author

but for actions that run on push would this value be empty?

From what I understood the github.event.pull_request.user.login just looks for PR author. Here is the sample run https://github.com/prudhvigodithi/opensearch-build/actions/runs/9100626642 that was triggered for a push in open PR prudhvigodithi/opensearch-build#104, still the github.event.pull_request.user.login was identified as author of the PR.
@bbarani @peterzhuamazon

@prudhvigodithi
Copy link
Member Author

Can you also update this instance as well?

Updated the Dependabot PR actions as well.

Copy link
Contributor

❌ Gradle check result for 82ab1bd: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 5dcc4c1: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@deshsidd
Copy link
Contributor

@prudhvigodithi Is this ready for review/merge or still in draft?

@prudhvigodithi prudhvigodithi marked this pull request as ready for review May 15, 2024 19:52
@prudhvigodithi prudhvigodithi requested a review from peternied as a code owner May 15, 2024 19:52
@prudhvigodithi
Copy link
Member Author

prudhvigodithi commented May 15, 2024

@prudhvigodithi Is this ready for review/merge or still in draft?
Ya its ready for review @deshsidd.

@prudhvigodithi prudhvigodithi requested a review from reta May 15, 2024 19:53
@cwperks
Copy link
Member

cwperks commented May 15, 2024

Tests with failures:

  • org.opensearch.rest.action.admin.cluster.RestNodesStatsActionTests.testIndexMetricsRequestWithoutIndicesMetric

@prudhvigodithi
Copy link
Member Author

prudhvigodithi commented May 15, 2024

The test org.opensearch.rest.action.admin.cluster.RestNodesStatsActionTests.testIndexMetricsRequestWithoutIndicesMetric is coming from class RestNodesStatsActionTests, check the metrics dashboards and I can see this as flaky.

Copy link
Contributor

❌ Gradle check result for 5dcc4c1: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@prudhvigodithi
Copy link
Member Author

prudhvigodithi commented May 15, 2024

The failure error is

 What went wrong:
Execution failed for task ':distribution:bwc:staged:buildBwcLinuxTar'.
> Building 2.14.0 didn't generate expected file /var/jenkins/workspace/gradle-check/search/distribution/bwc/staged/build/bwc/checkout-2.14/distribution/archives/linux-tar/build/distributions/opensearch-min-2.14.0-SNAPSHOT-linux-x64.tar.gz

Copy link
Contributor

❌ Gradle check result for 5dcc4c1: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

Copy link
Contributor

❌ Gradle check result for 5dcc4c1: null

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@peternied
Copy link
Member

@prudhvigodithi Please follow up on the past 2 failed checks as indicated by the checklist:

Failing checks are inspected and point to the corresponding known issue(s) (See: Troubleshooting Failing Builds)

@prudhvigodithi
Copy link
Member Author

@prudhvigodithi Please follow up on the past 2 failed checks as indicated by the checklist:

Thanks @peternied but while creating PR initially I did that, however I did it again and force pushed. I will and watch the gradle check.

Copy link
Contributor

✅ Gradle check result for cea2e08: SUCCESS

Copy link

codecov bot commented May 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.50%. Comparing base (b15cb0c) to head (cea2e08).
Report is 284 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #13688      +/-   ##
============================================
+ Coverage     71.42%   71.50%   +0.08%     
- Complexity    59978    61138    +1160     
============================================
  Files          4985     5059      +74     
  Lines        282275   287522    +5247     
  Branches      40946    41646     +700     
============================================
+ Hits         201603   205585    +3982     
- Misses        63999    64988     +989     
- Partials      16673    16949     +276     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@peternied peternied merged commit a9298c9 into opensearch-project:main May 15, 2024
28 checks passed
deshsidd pushed a commit to deshsidd/OpenSearch that referenced this pull request May 17, 2024
parv0201 pushed a commit to parv0201/OpenSearch that referenced this pull request Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants