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.

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

Updates to logging for artifact uploads #949

Merged
merged 6 commits into from
Nov 30, 2021

Conversation

konradpabjan
Copy link
Contributor

@konradpabjan konradpabjan commented Nov 29, 2021

This fixes a bunch of small things that make the upload process better

  • Clarifications around the upload size
  • Improved debug logs around gzip behavior
  • truncate millisecond time with error message during exponential back-off is in-progress
  • Improve logs to be be a bit more verbose
  • Links to how to enable debug logs + link to limitations around gzip behavior

Improved logs (with step debugs enabled): https://github.com/konradpabjan/artifact-test/runs/4359261863?check_suite_focus=true

image

Older logs for comparison: https://github.com/konradpabjan/artifact-test/runs/4266680264?check_suite_focus=true

image


Closes:
actions/upload-artifact#208

More clear error message to close out:
actions/upload-artifact#243

@konradpabjan konradpabjan marked this pull request as ready for review November 29, 2021 23:05
@konradpabjan konradpabjan requested review from a team November 29, 2021 23:05
Copy link

@pfleidi pfleidi left a comment

Choose a reason for hiding this comment

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

I had one minor question. Looks great otherwise!

await uploadHttpClient.patchArtifactSize(uploadResult.totalSize, name)

if (uploadResult.failedItems.length > 0) {
core.info(
Copy link

Choose a reason for hiding this comment

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

Could this be a warning or is this scenario expected to happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So there is a continueOnError option that we have available: https://github.com/actions/toolkit/tree/main/packages/artifact#available-options

It's not really used and we default to false for that so this is mainly for action authors that are doing some weird stuff. If that is true then it would error out earlier. The existing library also returns an array of failedItems: https://github.com/actions/toolkit/tree/main/packages/artifact#upload-result so it's basically up to authors to do whatever they want with that failed upload result list.

I lean on not creating a warning because it shows up as an annotation and in the past there was pushback about warning annotations we made if there were no files found with the input path

@konradpabjan konradpabjan merged commit 4df5abb into main Nov 30, 2021
@konradpabjan konradpabjan deleted the konradpabjan/misc-artifacts branch November 30, 2021 17:53
at-wat pushed a commit to at-wat/actions-toolkit that referenced this pull request Aug 31, 2023
* More details logs during artifact upload

* extra logging

* Updates to artifact logging + clarifications around upload size

* Fix linting errors

* Update packages/artifact/src/internal/artifact-client.ts

Co-authored-by: campersau <[email protected]>

Co-authored-by: campersau <[email protected]>
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.

3 participants