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

[Bug/Question]: wait-seconds-before-first-polling: '0' causes crash #994

Closed
Rawa opened this issue Dec 18, 2024 · 3 comments · Fixed by #995
Closed

[Bug/Question]: wait-seconds-before-first-polling: '0' causes crash #994

Rawa opened this issue Dec 18, 2024 · 3 comments · Fixed by #995
Assignees
Labels
enhancement New feature or request

Comments

@Rawa
Copy link

Rawa commented Dec 18, 2024

What happened?

I want to start polling right away since we wait for other jobs midway through our task. Thus I tried setting wait-seconds-before-first-polling: '0' since I don't want to waste 10 seconds if the other jobs are already completed. However this yields the following error on 3.5.0.

Is there a good reason for the 10 second wait default? Feels like quite an arbitrary number that probably should be off by default?

Error output:

 file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:29146
          const error2 = new ZodError(ctx.common.issues);
                         ^
  
  _ZodError: [
    {
      "code": "custom",
      "message": "Too short interval for pollings",
      "path": []
    }
  ]
      at get error [as error] (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:29146:24)
      at ZodEffects.parse (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:29250:18)
      at getDuration (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:32585:21)
      at Object.transform (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:32567:91)
      at ZodEffects._parse (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:32070:31)
      at ZodEffects._parseSync (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:29236:25)
      at ZodEffects.safeParse (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:29266:25)
      at ZodEffects.parse (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:29247:25)
      at parseInput (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:32695:35)
      at run (file:///__w/_actions/kachick/wait-other-jobs/v3.5.0/dist/index.js:34137:54) {
    issues: [
      {
        code: 'custom',
        message: 'Too short interval for pollings',
        path: []
      }
    ],
    addIssue: [Function (anonymous)],
    addIssues: [Function (anonymous)],
    errors: [
      {
        code: 'custom',
        message: 'Too short interval for pollings',
        path: []
      }
    ]
  }
  
  Node.js v20.18.0

Version

v3.5.0

Permissions

Parameters

Relevant log output

-
@Rawa Rawa added bug Something isn't working triage labels Dec 18, 2024
@Rawa Rawa changed the title [Bug/Question]: [Bug/Question]: wait-seconds-before-first-polling: '0' causes crash Dec 18, 2024
@Rawa Rawa changed the title [Bug/Question]: wait-seconds-before-first-polling: '0' causes crash [Bug/Question]: wait-seconds-before-first-polling: '0' causes crash Dec 18, 2024
@kachick kachick added enhancement New feature or request and removed bug Something isn't working triage labels Dec 19, 2024
@kachick kachick added this to 🛸 Dec 19, 2024
@github-project-automation github-project-automation bot moved this to 🙋‍♂ in 🛸 Dec 19, 2024
@kachick kachick moved this from 🙋‍♂ to 💪 in 🛸 Dec 19, 2024
@kachick
Copy link
Owner

kachick commented Dec 19, 2024

I tried setting wait-seconds-before-first-polling: '0'

Blocking zero for the first waiting has just come from keeping a simple implementation.
There are no other reasons to limit the feature, so I have sent a PR in GH-995.
I will release it in the next minor release.

Is there a good reason for the 10 second wait default? Feels like quite an arbitrary number that probably should be off by default?

On the other hand, the default 10 seconds were adjusted for my use and experience, and there is one historical reason.
In the v1 era, I set the default polling method to exponential_backoff.
It often caused much longer waiting times. So extending the initial wait reduced the total time as a result.

However, it was hacky, so I switched the default method to equal_intervals since v2. ref: GH-596

I agree that a shorter value or the same as other pollings might be reasonable since preferred equal_intervals.
It might be changed in a major release.
I have added the TODO comments in action.yml.

@github-project-automation github-project-automation bot moved this from 💪 to 🎉 in 🛸 Dec 19, 2024
@Rawa
Copy link
Author

Rawa commented Dec 19, 2024

Thanks for the quick fix 👍

@kachick
Copy link
Owner

kachick commented Dec 19, 2024

Released in v3.6.0 https://github.com/kachick/wait-other-jobs/releases/tag/v3.6.0 🚀

Thanks for reporting this issue 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants