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

mining-proxy needs graceful death instead of panic #484

Closed
bcral opened this issue Mar 9, 2023 · 15 comments · Fixed by #1021
Closed

mining-proxy needs graceful death instead of panic #484

bcral opened this issue Mar 9, 2023 · 15 comments · Fixed by #1021
Assignees
Labels
good first issue Good for newcomers
Milestone

Comments

@bcral
Copy link
Contributor

bcral commented Mar 9, 2023

The mining-proxy seems to be panicking on closure. This makes test coverage data impossible to retrieve with llvm-cov.

This same issue used to exist on the pool and TProxy until it was fixed by #432

@bcral
Copy link
Contributor Author

bcral commented Mar 9, 2023

Please tag as "good first issue"!

@pavlenex pavlenex added the good first issue Good for newcomers label Mar 12, 2023
@Priceless-P
Copy link
Contributor

@bcral i’d like to work on this.

@Priceless-P
Copy link
Contributor

@bcral Just to be sure, this is the file that needs to be worked on for this issue right?

@bcral
Copy link
Contributor Author

bcral commented Mar 16, 2023

@Priceless-P Yes, that's correct.
You may want to check with @darricksee first - he may have already started on this.

@darricksee
Copy link
Collaborator

I haven't and probably won't get to it soon.

@darricksee
Copy link
Collaborator

Just a heads-up @Priceless-P - to implement this the status-channel also needs to be added to the mining-proxy before getting to this issue. You can certainly add both with this issue - take a look at how the status-channel is used in the pool role. It's basically a mechanism for the application to signal the main thread to terminate if one of the tasks has an error.

@Priceless-P
Copy link
Contributor

Okay. Thank you for letting me know. @darricksee

@SumantxD
Copy link

@bcral i would like to work on this issue, can you assign it to me.

@Priceless-P
Copy link
Contributor

Hi @SumantxD, I am currently working on it.

@SumantxD
Copy link

@Priceless-P ok fine

@mihir1739
Copy link

Hey is anyone working on this issue ? If not please assign this to me .

@pavlenex
Copy link
Collaborator

pavlenex commented Apr 6, 2023

Hey @mihir1739 it seems @Priceless-P is working on it here #518

@plebhash
Copy link
Collaborator

@Priceless-P what is the status on this? why was #518 closed without being merged?

@Priceless-P
Copy link
Contributor

@plebhash Yes.

@johnnyasantoss
Copy link
Contributor

Is this up for grabs? @Priceless-P

@GitGab19 GitGab19 linked a pull request Jul 3, 2024 that will close this issue
@pavlenex pavlenex added this to the 1.0.2 milestone Jul 16, 2024
@github-project-automation github-project-automation bot moved this from Todo 📝 to Done ✅ in SV2 Roadmap 🛣️ Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

8 participants