Skip to content

[None][infra] Use main as waive target branch in push event - #16397

Closed
weiminwang-nv wants to merge 2 commits into
NVIDIA:mainfrom
weiminwang-nv:infra/refine-waive-target-branch
Closed

[None][infra] Use main as waive target branch in push event#16397
weiminwang-nv wants to merge 2 commits into
NVIDIA:mainfrom
weiminwang-nv:infra/refine-waive-target-branch

Conversation

@weiminwang-nv

@weiminwang-nv weiminwang-nv commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved merge request pipeline handling for push-triggered builds.
    • TOT waive-list retrieval now correctly uses the main branch when source and target branches are identical.

Description

We trigger the L0_PostMerge pipeline by gitlab push webhook event. When we want to trigger the L0_PostMerge from a internal branch in gitlab repo, the “Merge Test Waive List” stage will fail in this case. Because the stage needs to read the TOT waive list of target branch from Github, while there is no such target branch in Github.

In this PR, we change the target branch of “Merge Test Waive List” stage to main branch when it's trigger by a push event.

Test Coverage

Before:
Build link

The “Merge Test Waive List” stage failed on fetch TOT waive list from Github:

image

With this PR:

Build link

image

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

Signed-off-by: Weimin Wang <301118019+weiminwang-nv@users.noreply.github.com>
@weiminwang-nv
weiminwang-nv requested a review from a team as a code owner July 15, 2026 02:04
@weiminwang-nv
weiminwang-nv requested review from mzweilz and niukuo July 15, 2026 02:04
@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Merge request pipeline

Layer / File(s) Summary
Handle push-trigger branch selection
jenkins/L0_MergeRequest.groovy
mergeWaiveList now selects main for TOT waive-list retrieval when the GitLab source and target branches are identical.

Estimated code review effort: 2 (Simple) | ~5 minutes

Suggested reviewers: mzweilz, niukuo

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the change, is concise, and follows the required [None][infra] format.
Description check ✅ Passed The description includes the required Summary, Description, Test Coverage, and PR Checklist sections with relevant details.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
jenkins/L0_MergeRequest.groovy (1)

1-1: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the required NVIDIA copyright header.

This modified file has no visible NVIDIA copyright header. As per coding guidelines, “Add the NVIDIA copyright header to all new files and update the copyright year on modified files.”

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@jenkins/L0_MergeRequest.groovy` at line 1, Add the standard NVIDIA copyright
header at the top of the modified Jenkins file, before the `@Library` declaration,
and update its year to the current year while preserving the existing pipeline
content.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@jenkins/L0_MergeRequest.groovy`:
- Around line 333-337: The targetBranch selection must only force "main" when
both env.gitlabTargetBranch and env.gitlabSourceBranch are set and equal. Update
the conditional around targetBranch in the shown pipeline block to require both
branch variables before applying the push-trigger override, while preserving the
existing fallback to globalVars[TARGET_BRANCH] otherwise.

---

Outside diff comments:
In `@jenkins/L0_MergeRequest.groovy`:
- Line 1: Add the standard NVIDIA copyright header at the top of the modified
Jenkins file, before the `@Library` declaration, and update its year to the
current year while preserving the existing pipeline content.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 31766bb5-6343-4826-aeef-1b3a849bd1bc

📥 Commits

Reviewing files that changed from the base of the PR and between e05790a and cf5ab99.

📒 Files selected for processing (1)
  • jenkins/L0_MergeRequest.groovy

Comment thread jenkins/L0_MergeRequest.groovy
Signed-off-by: Weimin Wang <301118019+weiminwang-nv@users.noreply.github.com>
@weiminwang-nv

Copy link
Copy Markdown
Contributor Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59330 [ run ] triggered by Bot. Commit: 91153df Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #59330 [ run ] completed with state SUCCESS. Commit: 91153df
/LLM/main/L0_MergeRequest_PR pipeline #47811 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

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.

2 participants