Skip to content

Conversation

@m1so
Copy link
Member

@m1so m1so commented Oct 30, 2025

Summary by CodeRabbit

  • Chores
    • Excluded Dependabot-generated PRs from several build and deployment workflow jobs to avoid redundant runs.
    • Restricted the security scan job to run only for base-repo contexts or non-pull_request events to reduce unnecessary execution.
    • Retained existing workflow behavior otherwise; no runtime behavior changes beyond the added gating.

@m1so m1so requested review from jamesbhobbs and mfranczel October 30, 2025 19:27
@m1so m1so requested a review from a team as a code owner October 30, 2025 19:27
@linear
Copy link

linear bot commented Oct 30, 2025

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

📝 Walkthrough

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "chore: Skip CD pipelines on Dependabot's PRs" accurately describes the primary changeset focus: the cd.yml modifications that gate six deployment jobs with a github.actor != 'dependabot[bot]' condition. The title is concise, specific, and directly reflects the main intent stated in the PR objectives. While the PR also includes a ci.yml modification to the gitleaks job (which is technically a CI component, not CD), this is a supplementary change that supports the broader goal without undermining the title's core accuracy.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c0ef479 and bbc5bd3.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: Build and push artifacts for Python 3.11
  • GitHub Check: Build and push artifacts for Python 3.12
  • GitHub Check: Build and push artifacts for Python 3.9
  • GitHub Check: Build and push artifacts for Python 3.10
  • GitHub Check: Test - Python 3.9
  • GitHub Check: Test - Python 3.11
  • GitHub Check: Test - Python 3.10
🔇 Additional comments (1)
.github/workflows/ci.yml (1)

217-227: Merge conflict resolved; fork-protection logic looks correct.

The previous merge conflict is now clean. The new if condition protects secrets by skipping the gitleaks job for fork PRs—only base-repo PRs and non-PR events (e.g., pushes to main) will run. This prevents GITHUB_TOKEN and GITLEAKS_LICENSE from being exposed in untrusted environments.

Per the enriched summary, Dependabot-specific skipping should be handled in cd.yml (via github.actor != 'dependabot[bot]' checks). Please verify that those complementary changes exist to complete the intent of this PR.


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

Copy link
Contributor

@jamesbhobbs jamesbhobbs left a comment

Choose a reason for hiding this comment

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

  1. conflicts, 2. I dont know why you're skipping dependabot, how will you know CD won't fail before merge to main?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 1b4d41d and 0ffc555.

📒 Files selected for processing (2)
  • .github/workflows/cd.yml (6 hunks)
  • .github/workflows/ci.yml (1 hunks)
🔇 Additional comments (6)
.github/workflows/ci.yml (1)

220-221: Gitleaks job missing Dependabot exclusion—inconsistent with CD pipeline changes.

The condition gates forks but doesn't exclude Dependabot, unlike the cd.yml jobs. If the intent is to skip all Dependabot bot activity, this should also check github.actor != 'dependabot[bot]'.

Verify if running gitleaks on Dependabot PRs is intentional (e.g., for security scanning updates) or an oversight.

.github/workflows/cd.yml (5)

33-34: Condition correctly gates build artifacts from forks and Dependabot.

The condition (github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request') && github.actor != 'dependabot[bot]' is sound: runs on base-repo PRs or non-PR events, excluding the bot.


148-148: Publish Python package correctly gates from forks and Dependabot.

Condition mirrors build-and-push-artifacts pattern consistently.


288-288: PyPI publish correctly requires tags and gates Dependabot.

Adds tag check (startsWith(github.ref, 'refs/tags/')) alongside actor check. Proper gating for production publishing.


312-312: Review app deployment correctly scoped to base-repo PRs, excluding Dependabot.

Simplified condition (no fork OR clause) because this job is PR-only by design. Consistent with deploy-only semantics.


357-357: Jupyter-for-local correctly gates from forks and Dependabot.

Condition matches build-and-push-artifacts pattern; consistent.

@m1so
Copy link
Member Author

m1so commented Oct 30, 2025

I dont know why you're skipping dependabot, how will you know CD won't fail before merge to main?

there's a pretty surprising behaviour with Dependabot & Github actions as the PRs are not considered forks, but Dependabot doesn't inherit Github secrets from the repository configuration (the secrets must be added manually)

@jamesbhobbs skipping CD is fine as any supply chain attack could exfiltrate Github tokens used for deploying

@github-actions
Copy link

github-actions bot commented Oct 30, 2025

📦 Python package built successfully!

  • Version: 1.0.0.dev8+1836c64
  • Wheel: deepnote_toolkit-1.0.0.dev8+1836c64-py3-none-any.whl
  • Install:
    pip install "deepnote-toolkit @ https://deepnote-staging-runtime-artifactory.s3.amazonaws.com/deepnote-toolkit-packages/1.0.0.dev8%2B1836c64/deepnote_toolkit-1.0.0.dev8%2B1836c64-py3-none-any.whl"

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

♻️ Duplicate comments (1)
.github/workflows/cd.yml (1)

125-126: Redundant Dependabot exclusion in status job (duplicate of past review comment).

The build-and-push-artifacts-status job depends on build-and-push-artifacts and repeats the github.actor != 'dependabot[bot]' check. Since the parent job already skips for Dependabot, the status job will never run in that case. The always() prefix causes the job to run anyway, only to fail the actor check—inefficient but harmless.

Consider: remove the actor check from this job's if condition and let the parent job's skip propagate, or remove always() if you want strict dependency on the parent's success.

Apply this diff if you prefer to rely on parent job's skip:

-    if: always() && (github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request') && github.actor != 'dependabot[bot]'
+    if: always() && (github.event.pull_request.head.repo.full_name == github.repository || github.event_name != 'pull_request')
📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 0ffc555 and c0ef479.

📒 Files selected for processing (2)
  • .github/workflows/cd.yml (6 hunks)
  • .github/workflows/ci.yml (1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml

219-219: could not parse as YAML: could not find expected ':'

(syntax-check)

🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml

[error] 220-220: syntax error: could not find expected ':'

(syntax)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Build and push artifacts for Python 3.10
  • GitHub Check: Build and push artifacts for Python 3.9
  • GitHub Check: Build and push artifacts for Python 3.11
  • GitHub Check: Build and push artifacts for Python 3.12
🔇 Additional comments (5)
.github/workflows/cd.yml (5)

34-34: Verify base-repo condition correctly gates Dependabot.

The condition properly skips the job for Dependabot PRs and fork PRs.

To confirm this gates Dependabot as intended, verify the logic:

  • github.event.pull_request.head.repo.full_name == github.repository: runs only for PRs from the base repo (not forks).
  • github.event_name != 'pull_request': runs for push/tag events.
  • github.actor != 'dependabot[bot]': skips when actor is Dependabot.

All three must be true to run. Dependabot PRs will fail the last condition and skip. ✓


147-148: Dependabot exclusion correctly gates publish-python-package.

The job is now properly skipped for Dependabot actors and fork PRs.


287-288: Dependabot exclusion correctly gates publish-to-pypi.

The job is now properly skipped for Dependabot and restricted to tag events.


311-312: Dependabot exclusion correctly gates deploy-review-app.

The job is now properly skipped for Dependabot actors and fork PRs. The explicit github.event_name == 'pull_request' ensures this only runs on PRs, which aligns with the review-app deployment purpose.


356-357: Dependabot exclusion correctly gates jupyter-for-local.

The job is now properly skipped for Dependabot actors and fork PRs.

@deepnote-bot
Copy link

deepnote-bot commented Oct 30, 2025

🚀 Review App Deployment Started

📝 Description 🌐 Link / Info
🌍 Review application ra-7
🔑 Sign-in URL Click to sign-in
📊 Application logs View logs
🔄 Actions Click to redeploy
🚀 ArgoCD deployment View deployment
Last deployed 2025-10-30 19:48:11 (UTC)
📜 Deployed commit 193337cfd90f2c507d870a06b2be90fc18ed066b
🛠️ Toolkit version 1836c64

@codecov
Copy link

codecov bot commented Oct 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.60%. Comparing base (1b4d41d) to head (bbc5bd3).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main       #7   +/-   ##
=======================================
  Coverage   76.60%   76.60%           
=======================================
  Files          99       99           
  Lines        5476     5476           
  Branches      748      748           
=======================================
  Hits         4195     4195           
  Misses       1281     1281           

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

@m1so m1so requested a review from jamesbhobbs October 30, 2025 20:13
@m1so m1so merged commit 4eb4cf0 into main Oct 31, 2025
31 checks passed
@m1so m1so deleted the michalbaumgartner/blu-5086-skip-cd-pipelines-on-depandabot-prs branch October 31, 2025 09:28
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.

6 participants