Skip to content

fix(deploy-candidate): Show app name in fetch error notification (backport #6622)#6624

Merged
siduck merged 1 commit into
masterfrom
mergify/bp/master/pr-6622
Jun 5, 2026
Merged

fix(deploy-candidate): Show app name in fetch error notification (backport #6622)#6624
siduck merged 1 commit into
masterfrom
mergify/bp/master/pr-6622

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented Jun 5, 2026

When the fetch error fires from send_build_instructions_and_add_build_steps (outside the "apps" stage), the else branch showed a generic message with no app name. The GithubFetchError already carries the app name in args[1], so extract it and include it in the notification message.


This is an automatic backport of pull request #6622 done by Mergify.

When the fetch error fires from send_build_instructions_and_add_build_steps
(outside the "apps" stage), the else branch showed a generic message with no
app name. The GithubFetchError already carries the app name in args[1], so
extract it and include it in the notification message.

(cherry picked from commit 70684d6)
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 5, 2026

Greptile Summary

Extracts the app name from GithubFetchError.args[1] in the else branch of update_with_app_not_fetchable, so the notification message names the failing app when the error fires outside the "apps" stage.

  • Adds a safe fallback (\"App \") when args[1] is absent, matching the guard already used in update_with_incompatible_node.
  • The new else branch now mirrors the if branch message format (<b>{app_name}</b> could not be fetched…) for consistency.

Confidence Score: 5/5

The change is a one-line extraction of an existing exception argument with a safe fallback; it cannot break the notification path.

The only code path touched is the else branch that previously emitted a generic, app-less message. The new code defensively checks len(exc.args) >= 2 before accessing args[1], matching the same guard already used in update_with_incompatible_node. Both GithubFetchError raise sites in app_source.py and app_release.py pass self.app as args[1], so the extraction is correct.

No files require special attention.

Important Files Changed

Filename Overview
press/press/doctype/deploy_candidate/deploy_notifications.py Adds app name extraction from GithubFetchError.args[1] in the else branch of update_with_app_not_fetchable; logic is correct and consistent with the if branch and similar handlers.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GithubFetchError raised] --> B{stage_slug is apps?}
    B -- Yes --> C[app_name from failed_step.step]
    B -- No --> D{exc.args has 2 items?}
    D -- Yes --> E[app_name from exc.args index 1]
    D -- No --> F[app_name is empty string]
    E --> G{app_name truthy?}
    F --> G
    G -- Yes --> H[app_str is bold app_name]
    G -- No --> I[app_str is App]
    C --> J[Notification with bold app name]
    H --> K[Notification with bold app name]
    I --> L[Notification with generic App text]
Loading

Reviews (1): Last reviewed commit: "fix(deploy-candidate): Show app name in ..." | Re-trigger Greptile

@siduck siduck merged commit 5804ba6 into master Jun 5, 2026
8 checks passed
@siduck siduck deleted the mergify/bp/master/pr-6622 branch June 5, 2026 05:38
@frappe-pr-bot
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 0.41.8 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 5, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@88553fa). Learn more about missing BASE report.
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...s/doctype/deploy_candidate/deploy_notifications.py 0.00% 3 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #6624   +/-   ##
=========================================
  Coverage          ?   49.97%           
=========================================
  Files             ?      955           
  Lines             ?    79069           
  Branches          ?      374           
=========================================
  Hits              ?    39511           
  Misses            ?    39532           
  Partials          ?       26           
Flag Coverage Δ
dashboard 60.33% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants