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.

Sign up for GitHub

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

Jump to bottom

Error handling for connection refused when attempting to use dashboard #68

Closed
HeeebsInc opened this issue Jan 21, 2023 · 5 comments · Fixed by #70
Closed

Error handling for connection refused when attempting to use dashboard #68

HeeebsInc opened this issue Jan 21, 2023 · 5 comments · Fixed by #70
Assignees
@sybrenjansen
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@HeeebsInc
Copy link

I want to start by saying this repository is sweet! Really love what you did here and we have been using this extensively for a lot of our operations

I have been dealing with a race condition that occurs when a dashboard is running in another container, and a process in a different container attempts to connect to it. I see the ConnectionRefused error which destroys the pipeline.

I've attempted to handle the error outside the scope of the package with no luck. Would it be possible to add a flag or handle the error in the package itself?

So if a connection attempt is made and fails, the multiprocessing pipeline continues without updating the pbar in the browser. We could also throw a warning

Let me know if I can help with this!
@sybrenjansen
Copy link
Owner

Thnx for the kind words! :)

In your situation, the dashboard in one container hasn't started yet, but the other process already wants to connect. Is that a correct summary of your race condition?

The change you suggest sounds good to me. And let's definitely throw a warning if that happens.

Is your error triggered in mpire.dashboard.manager.get_manager_client_dicts? This is called from mpire.progress_bar.ProgressBar._register_progress_bar. We can catch the error there, throw a warning, and return. That should be enough I believe. Perhaps you can try it out locally and check whether it works. If so, feel free to create a PR for it.

@sybrenjansen sybrenjansen added enhancement New feature or request good first issue Good for newcomers labels Jan 22, 2023
@HeeebsInc
Copy link
Author

Correct. There have been cases where the dashboard did start up in time but the connection between containers failed. I want to chalk it up to docker networking but I'm still troubleshooting. As I mentioned, the quick fix will be to throw a warning that the dashboard did not connect properly. We can even add a flag to force a connection if you think that would be helpful too

@sybrenjansen
Copy link
Owner

Adding a flag would be a useful addition, indeed.

I'm currently busy with some architectural changes in mpire. This will take a while to finish, so I can't work on this one in the near feature (few weeks). If you don't mind waiting, I'll integrate it with that release when I'm done. Otherwise, feel free to make a PR yourself in the mean time.

@sybrenjansen
Copy link
Owner

I want to go about it a bit differently.

If you want to use a dashboard you will have to call connect_to_dashboard first. This function actually only sets some parameters and the actual connecting to the dashboard happens later.

I want to change this such that this function will also check if it can establish a connection. If not, it will raise ConnectionRefused. In your code you can put a try/except around this function call, and then it shouldn't fail later on.

Is this a good solution for you as well?

@sybrenjansen sybrenjansen mentioned this issue Mar 1, 2023
Merged
@sybrenjansen sybrenjansen self-assigned this Mar 3, 2023
sybrenjansen added a commit that referenced this issue Mar 17, 2023
@sybrenjansen
Apply & apply async (#70)
d0e144d 
* Added the apply and apply_async functions (Fixes #63)
* When inside a Jupyter notebook, the progress bar will not automatically switch to a widget anymore. tqdm cannot always determine with certainty that someone is in a notebook or, e.g., a Jupyter console. Another reason is to avoid the many errors people get when having widgets or javascript disabled (Fixes #71)
* The dashboard.connect_to_dashboard function now raises a ConnectionRefused error when the dashboard isn't running, instead of silently failing and deadlocking the next map call with a progress bar (Fixes #68)
* Added support for a progress bar without knowing the size of the iterable. It used to disable the progress bar when the size was unknown
* Changed how max_tasks_active is handled. It now applies to the number of tasks that are currently being processed, instead of the number of chunks of tasks, as you would expect from the name. Previously, when the chunk size was set to anything other than 1, the number of active tasks could be higher than max_tasks_active
* Updated some exception messages and docs (Fixes #69)
* Changed how worker results, restarts, timeouts, unexpected deaths, and exceptions are handled. They are now handled by individual threads such that the main thread is more responsive. The API is the same, so no user changes are needed
* Mixing multiple map calls now raises an error (see docs)
* Fixed a bug where calling a map function with a progress bar multiple times in a row didn't display the progress bar correctly
* Fixed a bug where the dashboard didn't show an error when an exit function raised an exception

---------

Co-authored-by: sybrenjansen <sybren.jansen@gmail.com>
@sybrenjansen
Copy link
Owner

Released in v2.7.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sybrenjansen sybrenjansen

Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Apply & apply async sybrenjansen/mpire
2 participants
@sybrenjansen @HeeebsInc