Skip to content

Conversation

@liangyepianzhou
Copy link
Contributor

Motivation

The ci-pulsarbot.yaml is slightly out-dated, update pulsarbot with github-script

Verifying this change

test in liangyepianzhou#19

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Nov 4, 2025
xiangying added 2 commits November 4, 2025 17:08
fix
(cherry picked from commit 678c92b)
lhotari
lhotari previously approved these changes Nov 4, 2025
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari
Copy link
Member

lhotari commented Nov 4, 2025

/pulsarbot rerun-failure-checks

@lhotari lhotari changed the title [improve][ci] update pulsarbot with github-script [improve][ci] update pulsarbot to use github-script based implementation Nov 4, 2025
continue;
}
try {
await github.rest.actions.reRunWorkflow({
Copy link
Member

Choose a reason for hiding this comment

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

According to LLM, this should use reRunWorkflowFailedJobs instead of reRunWorkflow to rerun only the failed jobs.

Suggested change
await github.rest.actions.reRunWorkflow({
await github.rest.actions.reRunWorkflowFailedJobs({

Copy link
Member

Choose a reason for hiding this comment

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

@lhotari lhotari requested a review from Copilot November 4, 2025 15:27
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the external apache/pulsar-test-infra/pulsarbot@master action with an inline implementation using actions/github-script@v7. The change provides direct control over the pulsarbot functionality for managing PR workflow runs.

Key changes:

  • Migrated from external action to inline JavaScript implementation
  • Implements commands: /pulsarbot rerun, /pulsarbot rerun <jobname>, /pulsarbot stop/cancel, and /pulsarbot rerun-failure-checks
  • Adds comprehensive logging and error handling for workflow/job management operations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const arg = parts.length > 2 ? parts.slice(2).join(' ') : '';
const supported = ['rerun', 'stop', 'cancel', 'rerun-failure-checks'];
if (!supported.includes(sub)) {
console.log(`Unsupported command '${sub}'. Supported: '/pulsarbot rerun [jobName?]', '/pulsarbot stop', '/pulsarbot cancel'.`);
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The error message lists supported commands but omits 'rerun-failure-checks' which is included in the supported array on line 62. The message should include all supported commands or reference them generically.

Suggested change
console.log(`Unsupported command '${sub}'. Supported: '/pulsarbot rerun [jobName?]', '/pulsarbot stop', '/pulsarbot cancel'.`);
console.log(`Unsupported command '${sub}'. Supported: ${supported.map(cmd => `'/pulsarbot ${cmd}'`).join(', ')}.`);

Copilot uses AI. Check for mistakes.
Comment on lines 151 to 163
if (github.rest.actions.reRunJobForWorkflowRun) {
await github.rest.actions.reRunJobForWorkflowRun({
owner: context.repo.owner,
repo: context.repo.repo,
job_id: job.id
});
} else {
await github.request('POST /repos/{owner}/{repo}/actions/jobs/{job_id}/rerun', {
owner: context.repo.owner,
repo: context.repo.repo,
job_id: job.id
});
}
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

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

The fallback logic checks if reRunJobForWorkflowRun exists at runtime, but this adds unnecessary complexity. The GitHub REST API for rerunning jobs should be consistently available in the @v7 version of actions/github-script. Consider using only the primary method or documenting why the fallback is necessary.

Suggested change
if (github.rest.actions.reRunJobForWorkflowRun) {
await github.rest.actions.reRunJobForWorkflowRun({
owner: context.repo.owner,
repo: context.repo.repo,
job_id: job.id
});
} else {
await github.request('POST /repos/{owner}/{repo}/actions/jobs/{job_id}/rerun', {
owner: context.repo.owner,
repo: context.repo.repo,
job_id: job.id
});
}
await github.rest.actions.reRunJobForWorkflowRun({
owner: context.repo.owner,
repo: context.repo.repo,
job_id: job.id
});

Copilot uses AI. Check for mistakes.
if (fullRerunCount === 0) {
console.error(`No eligible workflow runs to re-run. Skipped running=${skippedRunning}, skipped by conclusion=${skippedConclusion}.`);
} else {
console.log(`Finished. Triggered full re-run for ${fullRerunCount} workflow run(s). Skipped running=${skippedRunning}, skipped by conclusion=${skippedConclusion}.`);
Copy link
Member

Choose a reason for hiding this comment

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

There is no intention to do a "full re-run". Please update this comment to be consistent with the intention about re-running failed jobs.

Copy link
Member

Choose a reason for hiding this comment

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

@lhotari lhotari dismissed their stale review November 4, 2025 15:36

Check re-running of failed jobs only

env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
uses: apache/pulsar-test-infra/pulsarbot@master
uses: actions/github-script@v7
Copy link
Member

Choose a reason for hiding this comment

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

Use most recent version, v8:

Suggested change
uses: actions/github-script@v7
uses: actions/github-script@v8

Comment on lines 88 to 104
let page = 1;
const allRunsRaw = [];
while (true) {
const { data } = await github.rest.actions.listWorkflowRunsForRepo({
owner: context.repo.owner,
repo: context.repo.repo,
actor: prUser,
branch: prBranch,
per_page: 100,
page
});
const wr = data.workflow_runs || [];
if (wr.length === 0) break;
allRunsRaw.push(...wr);
if (wr.length < 100) break;
page++;
}
Copy link
Member

Choose a reason for hiding this comment

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

Octokit has built-in support for pagination and retrieving all results in a single call: https://github.com/octokit/octokit.js?tab=readme-ov-file#pagination (check the last example for the single call)

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.33%. Comparing base (fefe771) to head (d74f9cd).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24940       +/-   ##
=============================================
+ Coverage     38.46%   74.33%   +35.86%     
- Complexity    13239    33535    +20296     
=============================================
  Files          1856     1913       +57     
  Lines        145283   149446     +4163     
  Branches      16876    17362      +486     
=============================================
+ Hits          55890   111088    +55198     
+ Misses        81827    29505    -52322     
- Partials       7566     8853     +1287     
Flag Coverage Δ
inttests 26.38% <ø> (+0.12%) ⬆️
systests 22.73% <ø> (+<0.01%) ⬆️
unittests 73.85% <ø> (+39.15%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 1417 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@liangyepianzhou
Copy link
Contributor Author

liangyepianzhou commented Dec 30, 2025

@lhotari The above comments had been fixed. do you have time take a look?
I have test it in liangyepianzhou#19 (comment)

Comment on lines -1 to -19
#
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
#

Copy link
Member

Choose a reason for hiding this comment

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

Please don't remove the license header

Comment on lines +115 to +131
async function listAllJobs(runId) {
const jobs = [];
let p = 1;
while (true) {
const { data } = await github.rest.actions.listJobsForWorkflowRun({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: runId,
per_page: 100,
page: p,
});
const js = data.jobs || [];
if (js.length === 0) break;
jobs.push(...js);
if (js.length < 100) break;
p++;
}
Copy link
Member

Choose a reason for hiding this comment

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

could we use github.paginate also in this case?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

per_page: 100,
},
);
console.log(`DEBUG runs for head_sha=${headSha}: total_count=${runsAtHeadRaw.total_count}, returned=${(runsAtHeadRaw.workflow_runs||[]).length}`);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The debug log references total_count property which does not exist on the paginated response. The github.paginate method returns an array of items directly, not an object with a total_count property. This will log undefined for the total_count value.

Suggested change
console.log(`DEBUG runs for head_sha=${headSha}: total_count=${runsAtHeadRaw.total_count}, returned=${(runsAtHeadRaw.workflow_runs||[]).length}`);
console.log(`DEBUG runs for head_sha=${headSha}: runs_returned=${runsAtHeadRaw.length}`);

Copilot uses AI. Check for mistakes.
},
);
console.log(`DEBUG runs for head_sha=${headSha}: total_count=${runsAtHeadRaw.total_count}, returned=${(runsAtHeadRaw.workflow_runs||[]).length}`);
const runsAtHead = runsAtHeadRaw.filter(r => r && typeof r === 'object');
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The filter on line 84 is redundant. The github.paginate method already returns an array of workflow run objects. This filter checking if items are objects doesn't add value and could be removed for cleaner code.

Suggested change
const runsAtHead = runsAtHeadRaw.filter(r => r && typeof r === 'object');
const runsAtHead = runsAtHeadRaw;

Copilot uses AI. Check for mistakes.
Comment on lines +116 to +131
const jobs = [];
let p = 1;
while (true) {
const { data } = await github.rest.actions.listJobsForWorkflowRun({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: runId,
per_page: 100,
page: p,
});
const js = data.jobs || [];
if (js.length === 0) break;
jobs.push(...js);
if (js.length < 100) break;
p++;
}
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The manual pagination implementation for listing jobs is unnecessary. The github.paginate utility method can be used here instead, which would simplify the code and reduce the risk of pagination bugs.

Suggested change
const jobs = [];
let p = 1;
while (true) {
const { data } = await github.rest.actions.listJobsForWorkflowRun({
owner: context.repo.owner,
repo: context.repo.repo,
run_id: runId,
per_page: 100,
page: p,
});
const js = data.jobs || [];
if (js.length === 0) break;
jobs.push(...js);
if (js.length < 100) break;
p++;
}
const jobs = await github.paginate(
github.rest.actions.listJobsForWorkflowRun,
{
owner: context.repo.owner,
repo: context.repo.repo,
run_id: runId,
per_page: 100,
},
(response) => response.data.jobs || []
);

Copilot uses AI. Check for mistakes.
Comment on lines +95 to +96
if (a.workflow_id !== b.workflow_id) return a.workflow_id - b.workflow_id;
return new Date(b.created_at) - new Date(a.created_at);
Copy link

Copilot AI Jan 7, 2026

Choose a reason for hiding this comment

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

The sorting logic uses numeric subtraction for workflow_id comparison, but workflow_id values can be very large numbers that might exceed JavaScript's safe integer range. This could lead to incorrect sorting. Use a comparison that works reliably with large numbers.

Suggested change
if (a.workflow_id !== b.workflow_id) return a.workflow_id - b.workflow_id;
return new Date(b.created_at) - new Date(a.created_at);
const aw = String(a.workflow_id);
const bw = String(b.workflow_id);
if (aw !== bw) {
if (aw.length < bw.length) return -1;
if (aw.length > bw.length) return 1;
return aw < bw ? -1 : 1;
}
const at = new Date(a.created_at).getTime();
const bt = new Date(b.created_at).getTime();
if (bt > at) return 1;
if (bt < at) return -1;
return 0;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc-required Your PR changes impact docs and you will update later.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants