Migrated resume code #3289

gapra-msft · 2025-11-11T17:54:26Z

Description

Refactor of jobs resume code for AzCopy as a library.

Feature / Bug Fix: (Brief description of the feature or issue being addressed)
Related Links:
Issues
Team thread
Documents
[Email Subject]

Type of Change

How Has This Been Tested?

Existing integration tests as this is a refactor.
Thank you for your contribution to AzCopy!

Copilot

Pull Request Overview

This PR refactors the jobs resume functionality in AzCopy to support library usage, moving resume-related code from the CLI layer into the azcopy package. The refactoring enables programmatic job resumption while maintaining CLI compatibility.

Key Changes

Migrated resume job logic from cmd/jobsResume.go to azcopy/jobsResume.go, exposing ResumeJob() as a public API
Introduced JobLifecycleManager to handle job lifecycle events with adaptive progress reporting
Moved credential utility functions to azcopy/credentialUtil.go for shared use between CLI and library modes

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
ste/mgr-JobPartMgr.go	Removed obsolete include/exclude transfer list check
ste/mgr-JobMgr.go	Removed include/exclude transfer methods and fields from job manager
jobsAdmin/init.go	Simplified ResumeJobOrder by removing SAS stripping and include/exclude logic
common/rpc-models.go	Removed obsolete SourceSAS, DestinationSAS, IncludeTransfer, and ExcludeTransfer fields from ResumeJobRequest
cmd/sync.go	Updated constant reference from base10Mega to azcopy.Base10Mega
cmd/root.go	Refactored Initialize function to handle library mode vs CLI mode initialization
cmd/jobsShow.go	Updated API calls to pass JobID as first parameter instead of in options
cmd/jobsResume.go	Completely refactored to use new azcopy.ResumeJob API with handler pattern
cmd/jobsRemove.go	Updated API call signature to pass JobID as first parameter
cmd/credentialUtil.go	Delegated credential logic to azcopy package functions
cmd/copyEnumeratorInit.go	Fixed trailing whitespace and variable reference
cmd/copy.go	Updated constant reference from base10Mega to azcopy.Base10Mega
cmd/constants.go	Removed constants migrated to azcopy package
cmd/cancel.go	Updated API call signature for GetJobSummary
azcopy/zt_credentialUtil_test.go	Moved and updated credential tests to azcopy package
azcopy/jobsShow.go	Updated API to take JobID as first parameter with cleaner option structs
azcopy/jobsResume.go	New file implementing ResumeJob API with handler pattern for progress tracking
azcopy/jobsRemove.go	Updated API signature to take JobID as first parameter
azcopy/jobLifecycleManager.go	New file implementing job lifecycle management with adaptive progress reporting
azcopy/credentialUtil.go	New file consolidating credential utility functions for library usage
azcopy/client.go	Added LogLevel support and moved constants from cmd package

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

Copilot · 2025-11-11T17:59:40Z

azcopy/jobLifecycleManager.go

+// TODO : rename interface method to OnError to match other method naming conventions
+func (j *JobLifecycleManager) Error(err string) {


[nitpick] The TODO comment on line 105 mentions renaming the interface method to OnError to match naming conventions, but this hasn't been addressed. This should either be implemented or the TODO should be tracked elsewhere to ensure consistency across the API.

Suggested change

// TODO : rename interface method to OnError to match other method naming conventions

func (j *JobLifecycleManager) Error(err string) {

// Renamed to OnError to match other method naming conventions

func (j *JobLifecycleManager) OnError(err string) {

Copilot · 2025-11-11T17:59:40Z

azcopy/credentialUtil.go

+	}
+	return false, nil
+}
+


Missing documentation for the exported function GetCredentialTypeForLocation. Public API functions should have doc comments explaining their parameters, return values, and behavior.

Suggested change

// GetCredentialTypeForLocation determines the appropriate credential type to use for a given storage location and resource.

//

// Parameters:

// - ctx: The context for cancellation and deadlines.

// - location: The storage location (e.g., Azure, local, etc.).

// - resource: The resource string representing the target or source.

// - isSource: True if the resource is a source; false if it is a destination.

// - cpkOptions: Options for customer-provided keys.

// - uotm: User OAuth token manager for managing OAuth tokens.

//

// Returns:

// - credType: The determined credential type for accessing the resource.

// - isPublic: True if the resource is publicly accessible; false otherwise.

// - err: Any error encountered during determination.

cmd/jobsResume.go

Copilot · 2025-11-11T17:59:41Z

azcopy/jobsResume.go

+type ResumeJobHandler interface {
+	OnStart(ctx JobContext)
+	OnTransferProgress(progress ResumeJobProgress)
+}


Missing documentation for the exported ResumeJobHandler interface. Public API interfaces should have doc comments describing their contract and when they will be called.

Copilot · 2025-11-11T17:59:41Z

azcopy/jobLifecycleManager.go

+type JobProgressTracker interface {
+	// Start - calls OnStart
+	Start()
+	// CheckProgress checks the progress of the job and returns the number of transfers completed so far and whether the job is done
+	CheckProgress() (uint32, bool)
+	// CompletedEnumeration checks whether the enumeration is complete
+	CompletedEnumeration() bool // Whether we should prompt before cancelling
+	// GetJobID returns the JobID of the job being tracked
+	GetJobID() common.JobID
+	// GetElapsedTime returns the elapsed time since the job started
+	GetElapsedTime() time.Duration
+}


Missing documentation for the exported JobProgressTracker interface. Public API interfaces should have doc comments explaining the contract and typical usage patterns.

Copilot · 2025-11-11T17:59:42Z

azcopy/credentialUtil.go

+
+func warnIfSharedKeyAuthForDatalake() {
+	sharedKeyDeprecation.Do(func() {
+		common.GetLifecycleMgr().Warn(sharedKeyDeprecationMessage)


The reference to common.GetLifecycleMgr() may return nil in library mode, but the code immediately calls .Warn() on line 52. While there's a nil check in the function body at line 128, the warnIfSharedKeyAuthForDatalake function doesn't guard against this nil case and could panic.

Suggested change

common.GetLifecycleMgr().Warn(sharedKeyDeprecationMessage)

lcm := common.GetLifecycleMgr()

if lcm != nil {

lcm.Warn(sharedKeyDeprecationMessage)

}

Copilot · 2025-11-11T17:59:42Z

azcopy/credentialUtil.go

+		common.GetLifecycleMgr().Info(message)
+	}


The call to common.GetLifecycleMgr().Info(message) on line 220 may panic if GetLifecycleMgr() returns nil, which can happen in library mode. A nil check is needed before calling the method.

Suggested change

common.GetLifecycleMgr().Info(message)

}

if mgr := common.GetLifecycleMgr(); mgr != nil {

mgr.Info(message)

}

Copilot · 2025-11-11T17:59:43Z

azcopy/credentialUtil.go

+			return
+		}
+
+		if err = checkAuthSafeForTarget(credType, resource.Value, TrustedSuffixes, location); err != nil {


In the check for TrustedSuffixes on line 330, the code references a global variable TrustedSuffixes that may not be set in the azcopy package context. The checkAuthSafeForTarget function receives extraSuffixesAAD as a parameter but on line 330 it references the global TrustedSuffixes variable. This should use a local reference or be passed as a parameter to ensure it has the correct value.

Suggested change

if err = checkAuthSafeForTarget(credType, resource.Value, TrustedSuffixes, location); err != nil {

if err = checkAuthSafeForTarget(credType, resource.Value, TrustedSuffixesAAD, location); err != nil {

Copilot · 2025-11-11T17:59:43Z

azcopy/jobsResume.go

+type ResumeJobProgress struct {
+	common.ListJobSummaryResponse
+	Throughput  float64
+	ElapsedTime time.Duration
+}


Missing documentation for the exported ResumeJobProgress struct. Public API types should have doc comments explaining their purpose, especially since this is part of the library API migration.

Copilot · 2025-11-11T17:59:43Z

azcopy/jobsResume.go

+type ResumeJobResult struct {
+	common.ListJobSummaryResponse
+	ElapsedTime time.Duration
+}


Missing documentation for the exported ResumeJobResult struct. Public API types should have doc comments explaining what they represent.

gapra-msft added 7 commits October 24, 2025 13:26

Remove required parameters from options bag

0751988

Move user input erroring to cmd layer

2dcf812

simplify SAS normalization

7d2cb0c

migrated the initialization portion of jobs resujme

e9478fa

migrated progress tracking of jobs resume

e786cae

add job mgr cleanup

a8f611c

handler is nil

a0219c4

Copilot AI review requested due to automatic review settings November 11, 2025 17:54

gapra-msft requested review from adreed-msft, dphulkar-msft, seanmcc-msft, souravgupta-msft, tanyasethi-msft, vibhansa-msft and wonwuakpa-msft as code owners November 11, 2025 17:54

gapra-msft changed the title ~~Gapra/resume migration~~ Migrated resume code Nov 11, 2025

Copilot started reviewing on behalf of gapra-msft November 11, 2025 17:55 View session

Merge branch 'main' into gapra/resumeMigration

e88cf9c

gapra-msft assigned dphulkar-msft Nov 11, 2025

Copilot finished reviewing on behalf of gapra-msft November 11, 2025 17:57

Copilot AI reviewed Nov 11, 2025

View reviewed changes

gapra-msft assigned adreed-msft and wonwuakpa-msft Nov 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Migrated resume code #3289

Migrated resume code #3289

Uh oh!

gapra-msft commented Nov 11, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Copilot AI Nov 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

		// TODO : rename interface method to OnError to match other method naming conventions
		func (j *JobLifecycleManager) Error(err string) {

+// GetCredentialTypeForLocation determines the appropriate credential type to use for a given storage location and resource.
+//
+// Parameters:
+//   - ctx: The context for cancellation and deadlines.
+//   - location: The storage location (e.g., Azure, local, etc.).
+//   - resource: The resource string representing the target or source.
+//   - isSource: True if the resource is a source; false if it is a destination.
+//   - cpkOptions: Options for customer-provided keys.
+//   - uotm: User OAuth token manager for managing OAuth tokens.
+//
+// Returns:
+//   - credType: The determined credential type for accessing the resource.
+//   - isPublic: True if the resource is publicly accessible; false otherwise.
+//   - err: Any error encountered during determination.

-		common.GetLifecycleMgr().Warn(sharedKeyDeprecationMessage)
+		lcm := common.GetLifecycleMgr()
+		if lcm != nil {
+			lcm.Warn(sharedKeyDeprecationMessage)
+		}

-		common.GetLifecycleMgr().Info(message)
-	}
+		if mgr := common.GetLifecycleMgr(); mgr != nil {
+			mgr.Info(message)
+		}

	if err = checkAuthSafeForTarget(credType, resource.Value, TrustedSuffixes, location); err != nil {
	if err = checkAuthSafeForTarget(credType, resource.Value, TrustedSuffixesAAD, location); err != nil {

Migrated resume code #3289

Are you sure you want to change the base?

Migrated resume code #3289

Uh oh!

Conversation

gapra-msft commented Nov 11, 2025

Description

Type of Change

How Has This Been Tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes

Reviewed Changes

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants