Skip to content

add support for manually setting outcome#21

Merged
alexrashed merged 4 commits intomainfrom
fix-job-reporting
Apr 24, 2024
Merged

add support for manually setting outcome#21
alexrashed merged 4 commits intomainfrom
fix-job-reporting

Conversation

@alexrashed
Copy link
Copy Markdown
Member

@alexrashed alexrashed commented Apr 23, 2024

Motivation

We have seen some instances in the past where the usage of this action was causing some issues, if the action was used in sub-workflows (which were triggered with the workflow_call trigger).
This is because the action really goes through all the jobs in the (complete, in this composite) workflow and checks all the outcome.
This means that this action can:

  • Report failure even though all jobs in the specific workflow where it's defined were successful.
  • Not be triggered if one uses the "rerun from failed" feature of GitHub actions, because the reporting step obviously does not define the failed step - defined in a completely different workflow - is not a dependency (defined by "needs").

Unfortunately, it's not possible to access the needs context from within a GitHub action (which would allow filtering the jobs to check for the ones the action depends on).
This is why this PR provides a different solution: It allows the user of the action to manually set the outcome (instead of letting the action detect the outcome). At the level of the action definition, the needs context can be accessed and can be used in a simple evaluation over all declared dependencies as follows:

${{ (contains(needs.*.result, 'failure') && 'failure') || 'success' }}

Which means the issue can be circumvented with a usage of the action like this:

  - name: Push to Tinybird
    if: always()
    uses: localstack/tinybird-workflow-push@v3
    with:
      github_token: ${{ secrets.GITHUB_TOKEN }}
      tinybird_token: ${{ secrets.TINYBIRD_TOKEN }}
      tinybird_datasource: <your-data-source>
      workflow_id: <custom-workflow-id>
      outcome: ${{ (contains(needs.*.result, 'failure') && 'failure') || 'success' }}

Changes

  • Provides a new optional parameter (non-breaking change) to manually set the outcome which should be reported to Tinybird.

Testing

  • We tested this change in one of our private repos where we saw this issue regularly.

@alexrashed alexrashed added the enhancement New feature or request label Apr 23, 2024
@alexrashed alexrashed requested a review from silv-io April 23, 2024 12:00
@alexrashed alexrashed self-assigned this Apr 23, 2024
@alexrashed alexrashed marked this pull request as ready for review April 24, 2024 14:36
Copy link
Copy Markdown
Member

@silv-io silv-io left a comment

Choose a reason for hiding this comment

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

LGTM! Just some alternative idea to how to solve this. But the chosen fix is good as well.

@alexrashed alexrashed merged commit c90be38 into main Apr 24, 2024
@alexrashed alexrashed deleted the fix-job-reporting branch April 24, 2024 15:37
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

None yet

Development

Successfully merging this pull request may close these issues.

2 participants