Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Code Change] AzCopy Error Improvements #2647

Merged
merged 12 commits into from
May 16, 2024
Merged

[Code Change] AzCopy Error Improvements #2647

merged 12 commits into from
May 16, 2024

Conversation

siminsavani-msft
Copy link
Contributor

@siminsavani-msft siminsavani-msft commented Apr 11, 2024 •
edited
Loading

Added Error Code URL as seen below, proposed by Daniel
azcopy copy "sourceurl" "destinationurl"

failed to perform copy command due to error: cannot start job due to error: cannot list files due to reason GET https://siminmaintest.blob.core.windows.net/test
--------------------------------------------------------------------------------
RESPONSE 401: 401 Server failed to authenticate the request. Please refer to the information in the www-authenticate header.
ERROR CODE: NoAuthenticationInformation
--------------------------------------------------------------------------------
<?xml version="1.0" encoding="utf-8"?><Error><Code>NoAuthenticationInformation</Code><Message>Server failed to authenticate the request. Please refer to the information in the www-authenticate header.
RequestId:36f56f01-101e-000f-124c-8c69c6000000
Time:2024-04-11T20:13:02.1634782Z</Message></Error>
--------------------------------------------------------------------------------
.
ERROR DETAILS: https://aka.ms/AzCopyError/NoAuthenticationInformation

Added error code URLs to commonly used commands: copy, sync, remove, and list.

  • E2E Tests added will only test NoAuthenticationInformation (and ResourceNotFound). Other error codes require extremely specific setups

@seanmcc-msft
Copy link
Member

@siminsavani-msft linked are active

@siminsavani-msft siminsavani-msft marked this pull request as ready for review May 6, 2024 15:21
@siminsavani-msft siminsavani-msft added this to the 10.25.0 milestone May 6, 2024
pranavmalik-msft
pranavmalik-msft reviewed May 6, 2024
View reviewed changes
Copy link
Contributor

@pranavmalik-msft pranavmalik-msft left a comment

Choose a reason for hiding this comment

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

You may want to update your branch since I've included the fix for the failing test case.

pranavmalik-msft
pranavmalik-msft suggested changes May 6, 2024
View reviewed changes
Copy link
Contributor

@pranavmalik-msft pranavmalik-msft left a comment

Choose a reason for hiding this comment

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

Can we have E2E testcases to verify this behavior

adreed-msft
adreed-msft approved these changes May 9, 2024
View reviewed changes
Copy link
Member

@adreed-msft adreed-msft left a comment

Choose a reason for hiding this comment

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

Looks good to me, questions about testing, though.

@adreed-msft
Copy link
Member

Another thought-- Does this catch things like a job failing after it begins running?

pranavmalik-msft
pranavmalik-msft reviewed May 10, 2024
View reviewed changes
Copy link
Contributor

@pranavmalik-msft pranavmalik-msft left a comment

Choose a reason for hiding this comment

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

Have we added tests for list, remove as we discussed ?

@siminsavani-msft
Copy link
Contributor Author

Have we added tests for list, remove as we discussed ?

Was working on other PRs, just added now!

@siminsavani-msft
Copy link
Contributor Author

Another thought-- Does this catch things like a job failing after it begins running?

Mostly catches the service side errors and spits out related shorthand error code URLs.

pranavmalik-msft
pranavmalik-msft approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@pranavmalik-msft pranavmalik-msft left a comment

Choose a reason for hiding this comment

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

LGTM

@siminsavani-msft siminsavani-msft merged commit 43eb15e into main May 16, 2024
17 checks passed
@siminsavani-msft siminsavani-msft deleted the siminsavani/error-codes branch May 16, 2024 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adreed-msft adreed-msft adreed-msft approved these changes

@pranavmalik-msft pranavmalik-msft pranavmalik-msft approved these changes

@gapra-msft gapra-msft Awaiting requested review from gapra-msft gapra-msft is a code owner

@nakulkar-msft nakulkar-msft Awaiting requested review from nakulkar-msft

@vibhansa-msft vibhansa-msft Awaiting requested review from vibhansa-msft vibhansa-msft is a code owner

@seanmcc-msft seanmcc-msft Awaiting requested review from seanmcc-msft seanmcc-msft is a code owner

Assignees

@adreed-msft adreed-msft

@gapra-msft gapra-msft

Labels
pr-ready-for-review
Projects
None yet
Milestone
10.25.0
Development

Successfully merging this pull request may close these issues.
5 participants
@siminsavani-msft @seanmcc-msft @adreed-msft @pranavmalik-msft @gapra-msft