Skip to content

fix(designer): Prevent deletionConnection occurring on Non-Arm Connections#8369

Merged
Eric-B-Wu merged 6 commits into
mainfrom
eric/preventDeletionNonArm
Oct 13, 2025
Merged

fix(designer): Prevent deletionConnection occurring on Non-Arm Connections#8369
Eric-B-Wu merged 6 commits into
mainfrom
eric/preventDeletionNonArm

Conversation

@Eric-B-Wu

@Eric-B-Wu Eric-B-Wu commented Oct 13, 2025

Copy link
Copy Markdown
Contributor

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

We delete the created connection for every newly created connection that errors. However, this delete is also happening for local connections as well. With connectors such as sql having both builtin and managed connectors, this would make it so that a failed builtin in sql connection could delete a managed connection. To prevent this from happening because we don't create builtin connections in ARM we can prevent the deletion from happening.

Impact of Change

  • Users: If users fail to create a builtin connection ex sql service provider connection, no delete connection occurs
  • Developers: Slightly modifying the delete path for creating connections
  • System: No system changes

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Contributors

@Eric-B-Wu

Screenshots/Videos

Copilot AI review requested due to automatic review settings October 13, 2025 20:09
@Eric-B-Wu Eric-B-Wu added the risk:low Low risk change with minimal impact label Oct 13, 2025

Copilot AI 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.

Pull Request Overview

This fix prevents unwanted connection deletions when builtin (non-ARM) connections fail during creation. The issue occurred because the error handling code was deleting connections regardless of whether they were ARM resources or local builtin connections, which could lead to unintended deletion of existing managed connections.

  • Extracts the ARM resource check into a variable for reuse
  • Conditionally deletes connections only for ARM resources in error handling
  • Prevents builtin connection failures from affecting managed connections

@github-actions

github-actions Bot commented Oct 13, 2025

Copy link
Copy Markdown
Contributor

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(designer): Prevent deletionConnection occurring on Non-Arm Connections
  • Issue: Title is clear, includes scope (designer) and describes both what and where the fix is applied.
  • Recommendation: None, your title is excellent—concise and specific.

Commit Type

  • Properly selected (fix), only one selected as required.
  • Only one box is checked, which is correct and ensures the PR is focused.

Risk Level

  • Clearly marked as Low—matches both template and label set on the PR.

What & Why

  • Current: We delete the created connection for every newly created connection that errors. However, this delete is also happening for local connections as well. With connectors such as sql having both builtin and managed connectors, this would make it so that a failed builtin in sql connection could delete a managed connection. To prevent this from happening because we don't create builtin connections in ARM we can prevent the deletion from happening.
  • Issue: None.
  • Recommendation: Section is filled with clear rationale for the code change.

Impact of Change

  • All required subsections filled with brief but meaningful details.
  • Recommendation:
    • Users: Clear about impact on failed connection deletion scenario.
    • Developers: Developers notified about a slight change to logic.
    • System: Explicitly states 'No system changes'.

Test Plan

  • Manual testing is marked as completed, which is acceptable for a low-risk fix of this scope.

⚠️ Contributors

  • Only the submitter is listed. While the section is not required, consider tagging others if PMs, designers, or additional reviewers contributed ideas.

Screenshots/Videos

  • Not provided, which is fine as the change is not visual in nature.

Summary Table

Section Status Recommendation
Title
Commit Type
Risk Level
What & Why
Impact of Change
Test Plan
Contributors ⚠️ Tag others if relevant, especially if they contributed ideas or reviews.
Screenshots/Videos

Your PR fully passes the title and template review! Just remember to always give credit to contributors in the future if teamwork occurs. Well done and thank you for maintaining high documentation quality!


Last updated: Mon, 13 Oct 2025 20:18:32 GMT

@Eric-B-Wu
Eric-B-Wu enabled auto-merge (squash) October 13, 2025 20:26
@Eric-B-Wu
Eric-B-Wu merged commit b693522 into main Oct 13, 2025
14 of 15 checks passed
@Eric-B-Wu
Eric-B-Wu deleted the eric/preventDeletionNonArm branch October 13, 2025 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants