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

add support for manually setting outcome #21

Merged
merged 4 commits into from
Apr 24, 2024
Merged

Conversation

alexrashed
Copy link
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
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.

README.md Outdated Show resolved Hide resolved
__tests__/main.test.ts Outdated Show resolved Hide resolved
src/main.ts Show resolved Hide resolved
@alexrashed alexrashed merged commit c90be38 into main Apr 24, 2024
13 checks passed
@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