deploy-pages icon indicating copy to clipboard operation
deploy-pages copied to clipboard

`error_count` input name is confusing

Open TWiStErRob opened this issue 2 years ago • 7 comments
trafficstars

    steps:
      - name: "Deploy to GitHub Pages"
        id: deployment
        uses: actions/deploy-pages@v2
        with:
          error_count: 0

I might've mis-interpreted the error_count when initially configuring the action. I want to allow no errors, however based on the below log this error_count seems to mean deployment_in_progress_poll_count or something like it.

Created deployment for b6872282209cb2c94d458dcc594dfddd82d733b9, ID: b6872282209cb2c94d458dcc594dfddd82d733b9
Getting Pages deployment status...
Current status: syncing_files
Error: Too many errors, aborting!
Error: Failed with status code: 0
Canceling Pages deployment...
Canceled deployment with ID b6872282209cb2c94d458dcc594dfddd82d733b9

TWiStErRob avatar Oct 25 '23 14:10 TWiStErRob

There's a brief parameter description included in the Action manifest:

https://github.com/actions/deploy-pages/blob/03b62d1911e004eb0f6d6930948c60702a75adb8/action.yml#L16-L17

Fair misunderstanding, especially since we shouldn't really allow error_count: 0. That causes the action to fail even if there are 0 errors when it does the check. 😅 🤦

The behavior you're describing would be achieved by error_count: 1, as in "fail on the first error".

I see you also found a similar write-up in:

  • https://github.com/actions/deploy-pages/issues/172#issuecomment-1779451594

We may consider revising the naming but, for now, error_count: 1 achieves the desired behavior.

Apologies for the confusion! ❤

JamesMGreene avatar Nov 20 '23 21:11 JamesMGreene

Thanks, makes sense.

It's the good old "threshold problem". Over at detekt we also had similar problems and they're being renamed in the upcoming major to clarify.

I'm curious if this issue will get more upvotes in the coming year, especially if/when this upload approach goes out of beta and adoption grows.

TWiStErRob avatar Nov 20 '23 23:11 TWiStErRob

I'm curious if this issue will get more upvotes in the coming year, especially if/when this upload approach goes out of beta and adoption grows.

Oh, we're waaaaay out of beta. 😅 This action is involved in more than 2 million Pages deployments per week.

We might be open to renaming the parameter in a future major release, though. 🤷 Any naming suggestions?

JamesMGreene avatar Nov 22 '23 23:11 JamesMGreene

I was referring to this "Beta": image

I think you said it best

error_count: 1, as in "fail on the first error".

i.e. fail_on_error_count: 5

I think maybe: allowed_status_failures: 4 would be an even better name. But it implies that during migration users have to subtract one from their config. Default can stay as it doesn't really matter if it fails on 10th or 11th.

Another alternative could be status_report_retries: 4.

TWiStErRob avatar Nov 23 '23 11:11 TWiStErRob

Sorry to be wishy-washy, but I'm going to reopen this so the feedback isn't lost. 😅

JamesMGreene avatar Nov 26 '23 17:11 JamesMGreene

Sorry to be wishy-washy, but I'm going to reopen this so the feedback isn't lost. 😅

Up to you how to handle this. Feedback is a weird thing, it's not always actionable 😇

TWiStErRob avatar Nov 27 '23 07:11 TWiStErRob

fail-at-error-number: or fail-at-error-count:

Note that github seems to favor kebab over snake case for parameters, so now would be a good time to change that too.

jsoref avatar Jan 02 '25 15:01 jsoref