Skip to content

Conversation

@gapra-msft
Copy link
Member

Description

Resolves #3273

With the NFS over REST changes, we were checking share protocol compatibility when we didnt have to. This is changing the behavior of validateProtocolCompatibility to just validate File/FileNFS locations.

  • Feature / Bug Fix: (Brief description of the feature or issue being addressed)

  • Related Links:

  • Issues

  • Team thread

  • Documents

  • [Email Subject]

Type of Change

  • Bug fix
  • New feature
  • Documentation update required
  • Code quality improvement
  • Other (describe):

How Has This Been Tested?

Added unit tests for the fix

Thank you for your contribution to AzCopy!

Copy link
Contributor

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 optimizes protocol compatibility validation for Azure File Shares by adding conditional checks to skip validation when File locations are not involved, and simplifies the validation logic. The changes prevent unnecessary service client operations for S3, GCP, Blob, and Local transfers.

Key changes:

  • Adds conditional guards to skip validation when neither source nor destination is a File location
  • Simplifies validateProtocolCompatibility() logic by removing complex switch statements
  • Fixes a potential nil service client bug in getShareProtocolType()
  • Adds comprehensive test coverage for the conditional validation logic

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
common/util.go Returns initialized ServiceClient struct instead of nil for unsupported locations in the default case
cmd/syncEnumerator.go Adds conditional check to skip protocol validation when File locations are not involved
cmd/copy.go Adds conditional check to skip protocol validation when File locations are not involved
cmd/flagsValidation.go Simplifies validation logic, adds nil check for service client, and fixes error handling
cmd/flagsValidation_test.go Adds comprehensive test coverage for the new conditional validation logic

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

AzCopy v10.31.0 segmentation violation copying from S3 to ADLS

4 participants