[DownloadBuildArtifactsV0] Add download post-check #14065

alexander-smolyakov · 2020-12-11T13:07:35Z

Task name:

DownloadBuildArtifactsV0

Description:
This PR implements an additional check of the downloaded artifact' items.

Changes:

Added the possibility to check the integrity of the downloaded artifact' items

Additions:

Introduced new task optional parameter - Check downloaded files (Can be found in advanced settings)
Introduced new task optional parameter - Retry count (Can be found in advanced settings)
Added handlerCheckDownloadedFiles function for validation results of artifact download
Added isItemCorrupted function for checking information from download ticket of artifact item
Added check of the file size in local storage

Refactoring:

The logic for download build artifacts was extracted to the download handlers classes
Introduced new interfaces to contain all needed information to generate Source and Destination providers
All download operations are now executed via the executeWithRetry handler

Others:

Bumped up task minor version to 183
Added loc strings for messages

UI Demonstration

Documentation changes required: Yes

Added unit tests: No

Attached related issue:

DownloadBuildArtifacts has incomplete downloads with no errors #13250

Checklist:

Task version was bumped - please check instruction how to do it
Checked that applied changes work as expected

- Move functions to separate module - Implement consistency checker

* Fix code formatting in the download_helper module * Added comments for the checkArtifactConsistency and isItemCorrupted functions * Move some debug messages to loc strings * Add additional checks to the isItemCorrupted function to prevent false alarms

Tasks/DownloadBuildArtifactsV0/download_helper.ts

MattGal · 2020-12-16T20:53:22Z

Can anyone give an ETA as to when this might be merged?

alexander-smolyakov · 2020-12-17T13:31:08Z

Can anyone give an ETA as to when this might be merged?

Hi @MattGal, I'm currently double-checking all changes to prevent any possible regressions for this task, so I believe we will merge these changes within 2-3 workdays.

- Revert refactoring changes - Introduce a new task option - "Check downloaded files" - Added check of the file size in local storage - Improve debug logging

alexander-smolyakov · 2020-12-24T09:51:06Z

Test run of the pipeline with these changes (with Check downloaded files option enabled): https://dev.azure.com/v-alsmo/ArtifactsTest/_build/results?buildId=768&view=results

Tasks/DownloadBuildArtifactsV0/download_helper.ts

Tasks/DownloadBuildArtifactsV0/main.ts

Tasks/DownloadBuildArtifactsV0/task.loc.json

- Fix default value of the option in task.lock.json - Change strategy of checking download file - Add commentary to isItemCorrupted function

MattGal · 2021-01-04T16:44:02Z

@alexander-smolyakov any update on when this might get merged?

safern · 2021-01-05T20:16:03Z

I see that the only code path that uses execWithRetries is a windows specific code path and when isZipDownloadDisabledBool==true. So this fix will only cause our issues to manifest on a different way by showing an error that the downloaded artifact is corrupt.

Can you add retry for the other code paths so that this task is more resilient?

Tasks/DownloadBuildArtifactsV0/download_handlers/handlers_implementation.ts

Tasks/DownloadBuildArtifactsV0/download_helper.ts

…dBuildArtifacts_incomplete_downloads

anatolybolshakov · 2021-01-15T16:37:20Z

Checked with lightweight artifact (38 b) - I think it make sense to test it with more heavy artifacts also (at least 200 mb)

hoyosjs · 2021-01-27T23:34:42Z

@alexander-smolyakov any updates on when this can get merged and deployed? It's hitting pretty often in our CI.

- Fix linter warnings - Split download handlers classes to separate files - Add commentaries to the code - Update log messages

alexander-smolyakov · 2021-01-28T09:17:39Z

Checked with lightweight artifact (38 b) - I think it make sense to test it with more heavy artifacts also (at least 200 mb)

@anatolybolshakov I tested my changes on heavyweight artifacts items (The summary size of Artifact is 24399 mb).

The build artifact that I used for the test: https://dev.azure.com/v-alsmo/ArtifactsTest/_build/results?buildId=929&view=artifacts&pathAsName=false&type=publishedArtifacts

Run of the test pipeline that I used for e2e testing: https://dev.azure.com/v-alsmo/ArtifactsTest/_build/results?buildId=939&view=results

alexander-smolyakov · 2021-01-28T11:08:25Z

@MattGal @hoyosjs currently we on the testing stage for this PR, we plan to ship this fix with the release for the 183rd sprint. Usually, it requires two or three weeks to deploy the release to all Azure DevOps users. This means that the fix will be available for all users around the 5th of March.

anatolybolshakov · 2021-01-29T11:27:03Z

Could it make sense to expose retry count as input also (in advanced inputs group)?

alexander-smolyakov · 2021-01-29T11:32:32Z

Could it make sense to expose retry count as input also (in advanced inputs group)?

Good point, I will add an additional input for it.
Thanks!

…dBuildArtifacts_incomplete_downloads

anatolybolshakov · 2021-01-29T17:41:30Z

Btw tested it with artifact from the same pipeline - LGTM for me 👍

EzzhevNikita

LGTM!
Changes tested with artifacts of about 500mb, found no issues (pipeline run)

anatolybolshakov

LGTM, thanks!

SamuelGuine · 2021-02-25T14:36:06Z

The UI "parallelizationLimit" input seem not used ? Same in previous version 0.178. Only the variable "release.artifact.download.parallellimit" works.

alexander-smolyakov · 2021-03-19T08:20:20Z

The UI "parallelizationLimit" input seem not used ? Same in previous version 0.178. Only the variable "release.artifact.download.parallellimit" works.

Hey @SamuelGuine, we fixed the issue with parallelizationLimit input (Link to related PR). This fix will be available in two or three weeks for all Azure DevOps users.

Aleksandr Smolyakov added 3 commits December 9, 2020 10:33

Add consistency checker

a799ba0

- Move functions to separate module - Implement consistency checker

Bump up task version

caa74c0

Polish the source code

ca78ba0

* Fix code formatting in the download_helper module * Added comments for the checkArtifactConsistency and isItemCorrupted functions * Move some debug messages to loc strings * Add additional checks to the isItemCorrupted function to prevent false alarms

alexander-smolyakov requested a review from a team December 11, 2020 13:07

alexander-smolyakov requested a review from narula0781 as a code owner December 11, 2020 13:07

alexander-smolyakov self-assigned this Dec 11, 2020

anatolybolshakov reviewed Dec 11, 2020

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_helper.ts Outdated Show resolved Hide resolved

EzzhevNikita approved these changes Dec 14, 2020

View reviewed changes

egor-bryzgalov approved these changes Dec 14, 2020

View reviewed changes

Aleksandr Smolyakov added 2 commits December 23, 2020 12:23

Revert changes

dab98c3

Resolve review points

f5df3d5

- Revert refactoring changes - Introduce a new task option - "Check downloaded files" - Added check of the file size in local storage - Improve debug logging

alexander-smolyakov requested review from anatolybolshakov, egor-bryzgalov, EzzhevNikita and a team December 24, 2020 09:40

anatolybolshakov reviewed Dec 24, 2020

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_helper.ts Outdated Show resolved Hide resolved

anatolybolshakov reviewed Dec 24, 2020

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_helper.ts Outdated Show resolved Hide resolved

anatolybolshakov reviewed Dec 24, 2020

View reviewed changes

Tasks/DownloadBuildArtifactsV0/main.ts Outdated Show resolved Hide resolved

anatolybolshakov reviewed Dec 24, 2020

View reviewed changes

Tasks/DownloadBuildArtifactsV0/main.ts Outdated Show resolved Hide resolved

EzzhevNikita reviewed Dec 24, 2020

View reviewed changes

Tasks/DownloadBuildArtifactsV0/task.loc.json Outdated Show resolved Hide resolved

resolve addressed review points

af9cc4c

- Fix default value of the option in task.lock.json - Change strategy of checking download file - Add commentary to isItemCorrupted function

MattGal mentioned this pull request Jan 5, 2021

Inability to unzip assets during build on Unix x64 dotnet/runtime#32805

Closed

Add retry logic

1881944

alexander-smolyakov requested review from anatolybolshakov and EzzhevNikita January 6, 2021 09:58

anatolybolshakov reviewed Jan 15, 2021

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_handlers/handlers_implementation.ts Outdated Show resolved Hide resolved

anatolybolshakov reviewed Jan 15, 2021

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_helper.ts Outdated Show resolved Hide resolved

anatolybolshakov reviewed Jan 15, 2021

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_helper.ts Outdated Show resolved Hide resolved

anatolybolshakov reviewed Jan 15, 2021

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_helper.ts Show resolved Hide resolved

anatolybolshakov reviewed Jan 15, 2021

View reviewed changes

Tasks/DownloadBuildArtifactsV0/download_helper.ts Outdated Show resolved Hide resolved

Merge branch 'master' into users/alexander-smolyakov/fix13250_Downloa…

0cbb1cf

…dBuildArtifacts_incomplete_downloads

Resolve review points

00164c3

- Fix linter warnings - Split download handlers classes to separate files - Add commentaries to the code - Update log messages

Resolve review point

aa28e85

Aleksandr Smolyakov added 2 commits January 29, 2021 16:54

Bump task version

c5bebb9

Merge branch 'master' into users/alexander-smolyakov/fix13250_Downloa…

344c510

…dBuildArtifacts_incomplete_downloads

Add input for retry count

2005cdc

alexander-smolyakov requested review from anatolybolshakov and a team February 1, 2021 09:49

EzzhevNikita approved these changes Feb 1, 2021

View reviewed changes

anatolybolshakov approved these changes Feb 1, 2021

View reviewed changes

egor-bryzgalov approved these changes Feb 2, 2021

View reviewed changes

alexander-smolyakov merged commit 44e3baa into master Feb 2, 2021

alexander-smolyakov mentioned this pull request Feb 2, 2021

DownloadBuildArtifacts has incomplete downloads with no errors #13250

Closed

MattGal mentioned this pull request Feb 22, 2021

Backport removing dead pwd code dotnet/runtime#48544

Merged

safern mentioned this pull request Mar 8, 2021

Enable CheckDownloadedFiles on DownloadBuildArtifacts task dotnet/runtime#49321

Merged

hoyosjs mentioned this pull request May 11, 2021

CI is currently broken "You cannot extract a file outside of the target path" dotnet/runtime#52596

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DownloadBuildArtifactsV0] Add download post-check #14065

[DownloadBuildArtifactsV0] Add download post-check #14065

alexander-smolyakov commented Dec 11, 2020 •

edited

Loading

MattGal commented Dec 16, 2020

alexander-smolyakov commented Dec 17, 2020

alexander-smolyakov commented Dec 24, 2020

MattGal commented Jan 4, 2021

safern commented Jan 5, 2021

anatolybolshakov commented Jan 15, 2021

hoyosjs commented Jan 27, 2021

alexander-smolyakov commented Jan 28, 2021

alexander-smolyakov commented Jan 28, 2021

anatolybolshakov commented Jan 29, 2021 •

edited

Loading

alexander-smolyakov commented Jan 29, 2021

anatolybolshakov commented Jan 29, 2021

EzzhevNikita left a comment

anatolybolshakov left a comment

SamuelGuine commented Feb 25, 2021

alexander-smolyakov commented Mar 19, 2021

[DownloadBuildArtifactsV0] Add download post-check #14065

[DownloadBuildArtifactsV0] Add download post-check #14065

Conversation

alexander-smolyakov commented Dec 11, 2020 • edited Loading

MattGal commented Dec 16, 2020

alexander-smolyakov commented Dec 17, 2020

alexander-smolyakov commented Dec 24, 2020

MattGal commented Jan 4, 2021

safern commented Jan 5, 2021

anatolybolshakov commented Jan 15, 2021

hoyosjs commented Jan 27, 2021

alexander-smolyakov commented Jan 28, 2021

alexander-smolyakov commented Jan 28, 2021

anatolybolshakov commented Jan 29, 2021 • edited Loading

alexander-smolyakov commented Jan 29, 2021

anatolybolshakov commented Jan 29, 2021

EzzhevNikita left a comment

Choose a reason for hiding this comment

anatolybolshakov left a comment

Choose a reason for hiding this comment

SamuelGuine commented Feb 25, 2021

alexander-smolyakov commented Mar 19, 2021

alexander-smolyakov commented Dec 11, 2020 •

edited

Loading

anatolybolshakov commented Jan 29, 2021 •

edited

Loading