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] github-workflows-require-timeout.json: timeout-minutes should allow expressions #354

Closed
webknjaz opened this issue Nov 21, 2023 · 4 comments · Fixed by #356
Closed
Labels
bug Something isn't working

Comments

@webknjaz
Copy link

$sbj. I want to have timeout-minutes: ${{ inputs.qemu && '60' || '20' }} set for a job. The inputs is a context that exists in reusable workflows. Here's the table of context availability FTR: https://docs.github.com/en/actions/learn-github-actions/contexts.

Here's how it fails for me:

  .github/workflows/reusable-build-wheel.yml::$.jobs.build-wheel: {'name': 'Build wheels on ${{ inputs.os }} ${{ inputs.qemu }}', 'runs-on': '${{ inputs.os }}-latest', 'timeout-minutes': '${{ inputs.qemu && 60 || 20 }}', 'steps': [{'name': 'Retrieve the project source from an sdist inside the GHA artifact', 'uses': 're-actors/checkout-python-sdist@release/v1', 'with': {'source-tarball-name': '${{ inputs.source-tarball-name }}', 'workflow-artifact-name': '${{ inputs.dists-artifact-name }}'}}, {'name': 'Set up QEMU', 'if': 'inputs.qemu', 'uses': 'docker/setup-qemu-action@v3', 'with': {'platforms': 'all'}, 'id': 'qemu'}, {'name': 'Prepare emulation', 'if': 'inputs.qemu', 'run': '# Build emulated architectures only if QEMU is set,\n# use default "auto" otherwise\necho "CIBW_ARCHS_LINUX=${{ inputs.qemu }}" >> "${GITHUB_ENV}"\n', 'shell': 'bash'}, {'name': 'Setup Python', 'uses': 'actions/setup-python@v4'}, {'name': 'Build wheels', 'uses': 'pypa/cibuildwheel@v2.16.2', 'env': {'CIBW_ARCHS_MACOS': 'x86_64 arm64 universal2'}}, {'name': 'Upload built artifacts for testing and publishing', 'uses': 'actions/upload-artifact@v3', 'with': {'name': '${{ inputs.dists-artifact-name }}', 'path': './wheelhouse/*.whl'}}]} is not valid under any of the given schemas
  Underlying errors caused this.

  Best Match:
    $.jobs.build-wheel: 'uses' is a required property
  Best Deep Match:
    $.jobs.build-wheel.timeout-minutes: '${{ inputs.qemu && 60 || 20 }}' is not of type 'number'

Approximate reusable workflow look:

# partial file, specific bit that is failing
on:
  workflow-call:
    inputs:
      ...
      qemu:
          default: ''
          ...
          required: false
          ...
      ...
...
jobs:
  job-id:
    ...
    timeout-minutes: ${{ inputs.qemu && '60' || '20' }}
    ...

Haven't looked into how to fix this, but the upstream github-workflow.json doesn't error out, so this custom schema must be too strict...

@sirosen sirosen added the bug Something isn't working label Nov 21, 2023
@sirosen
Copy link
Member

sirosen commented Nov 21, 2023

Yep; accepted! The schema was written with type: number. I'll put something in for expression evaluation strings and then it can be OR-ed together.

@webknjaz
Copy link
Author

This is where I'm trying to stick it, by the way: https://github.com/aio-libs/yarl/blob/3a5e605/.github/workflows/reusable-build-wheel.yml#L38.

sirosen added a commit that referenced this issue Nov 24, 2023
github-workflows-require-timeout now supports expression eval blocks
in the value for the timeout. Two test-cases ensure that such
workflows will pass, based on the example provided in #354

resolves #354

There is also a small refinement to test outputs here, which was added
while testing this addition.
@sirosen
Copy link
Member

sirosen commented Nov 25, 2023

I just got around to this today and handled it in v0.27.2, freshly released!
Please let me know if it does or doesn't work for you.

webknjaz added a commit to aio-libs/frozenlist that referenced this issue Dec 14, 2023
Previously, the jsonschema for non-raw number values was incomplete
but has been fixed in check-jsonschema v0.27.2.

Refs:

  * python-jsonschema/check-jsonschema#354
  * python-jsonschema/check-jsonschema#356
@webknjaz
Copy link
Author

@sirosen yes, thank you! Just got to verifying and it does work for me: aio-libs/frozenlist@a1c0e49 / aio-libs/frozenlist@3367693.

Though, there was one problem with my example above: the numbers should be unquoted. So it must be timeout-minutes: ${{ inputs.qemu && 60 || 20 }}. I don't know why I thought the quoted values worked — perhaps, it does work in non-reusable workflows or something changed.

Anyway, the schema looks good to me now.

webknjaz added a commit to aio-libs/yarl that referenced this issue Dec 15, 2023
Previously, the jsonschema for non-raw number values was incomplete
but has been fixed in check-jsonschema v0.27.2.

Refs:

  * python-jsonschema/check-jsonschema#354
  * python-jsonschema/check-jsonschema#356
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants