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

fix(workflow-hook): pre/post hook not sending output due to invalid key #3091

Merged
merged 1 commit into from
Feb 18, 2023

Conversation

Fabianoshz
Copy link
Contributor

@Fabianoshz Fabianoshz commented Feb 1, 2023

what

  • Fixes workflow output not sending output for ws channel.

why

  • Turns out if we set the first message as closed the Handle will close the channel before we send any message as can be seem here, we need to have at least 2 messages one with our output and another one to close the channel.

tests

  • I have tested my changes by ...

references

@Fabianoshz Fabianoshz requested a review from a team as a code owner February 1, 2023 14:17
@github-actions github-actions bot added the go Pull requests that update Go code label Feb 1, 2023
@@ -70,7 +70,8 @@ func (wh DefaultPostWorkflowHookRunner) Run(ctx models.WorkflowHookCommandContex
}
}

wh.OutputHandler.SendWorkflowHook(ctx, fmt.Sprintf("%s\n", string(out)), true)
wh.OutputHandler.SendWorkflowHook(ctx, string(out), false)
Copy link
Member

@nitrocode nitrocode Feb 1, 2023

Choose a reason for hiding this comment

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

Thanks for the contribution and fix @Fabianoshz. Could you please add the test section to the pr body to describe how this was tested e2e for post and pre workflpws?

Oh and could you verify that this works with automerge and no automerge?

And if possible, could you also add a unit test for this to ensure this doesn't break pre and post workflow runs going forward?

Copy link
Member

@nitrocode nitrocode Feb 10, 2023

Choose a reason for hiding this comment

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

cc: @Fabianoshz friendly ping

also have you tested this change out in your own setup?

@nitrocode nitrocode added this to the v0.23.0 milestone Feb 1, 2023
@nitrocode nitrocode changed the title fix: workflow output not sending output fix(workflow-hook): pre/post hook not sending output due to invalid key Feb 1, 2023
@nitrocode nitrocode added the waiting-on-response Waiting for a response from the user label Feb 10, 2023
Copy link
Member

@nitrocode nitrocode left a comment

Choose a reason for hiding this comment

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

Let's approve and get this fixed. We can always revert if there are issues

@nitrocode nitrocode merged commit 40f4e8c into runatlantis:main Feb 18, 2023
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
ijames-gc pushed a commit to gocardless/atlantis that referenced this pull request Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No output in atlantis/post_workflow_hook due to websocket 500 internal invalid key error
2 participants