-
Notifications
You must be signed in to change notification settings - Fork 690
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 workgroup AddContext bug #4783
Conversation
Signed-off-by: Joy Lal Chattaraj <8450903+chattarajoy@users.noreply.github.com>
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4783 +/- ##
=======================================
Coverage 76.04% 76.04%
=======================================
Files 140 140
Lines 16859 16861 +2
=======================================
+ Hits 12820 12822 +2
Misses 3787 3787
Partials 252 252
|
Thanks for the PR @chattarajoy, however what I would really like to do is drop the workgroup package entirely since it's only being used by test code at this point and I don't think we need our own custom goroutine manager anymore -- see #4206. Would you be interested in taking that work on? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep agreed, we should focus on possibly removing the package since it's only used in tests
Hi @skriss , since I was using this module in my own project (copied the code, not importing using go mod) I found this bug and fixed it in my project as well as thought of raising a fix here. I am sorry I won't be able to pick up the deletion of this model and remove its usage. |
@chattarajoy fair enough - I think we can merge this as-is and still look to do #4206 subsequently. |
In case functions are added to workgroup using AddContext method, the AddContext method doesn't exit if the corresponding function exits. This change makes sure the Run flow exits in case the function running has exited.