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 job service gRPC stream cancellation handling #3576

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Sovietaced
Copy link
Contributor

@Sovietaced Sovietaced commented May 10, 2024

This is a follow up to #3555 where I tried to get cute about handling stream cancellation. I updated the code from performing string matching to checking whether the error was of type context.Canceled and this introduced a cosmetic bug. The issue with this is that the errors returned in the stream are gRPC specific, so this pull request updates the unite test and fixes the logic to continue and not log the cancellation as some other error.

Signed-off-by: Sovietaced <sovietaced@gmail.com>
Signed-off-by: Sovietaced <sovietaced@gmail.com>
@Sovietaced Sovietaced marked this pull request as ready for review May 10, 2024 02:17
@Sovietaced Sovietaced changed the title Fix gRPC cancellation handling Fix job service gRPC cancellation handling May 10, 2024
@Sovietaced
Copy link
Contributor Author

Unit test failures seem unrelated

     🧪 - com/armadaproject/armada/internal/lookoutv2/repository | Failed
     🧪 - com/armadaproject/armada/internal/lookoutv2/repository | Failed
     🧪 - com/armadaproject/armada/internal/lookoutv2/repository | Failed

@Sovietaced Sovietaced changed the title Fix job service gRPC cancellation handling Fix job service gRPC stream cancellation handling May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant