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

Attempt to fix test_preload_remote_module on windows #3775

Merged
merged 5 commits into from
May 6, 2020

Conversation

jrbourbeau
Copy link
Member

No description provided.

@mrocklin
Copy link
Member

mrocklin commented May 5, 2020

Thank you for working to resolve this @jrbourbeau . I appreciate it.

@mrocklin
Copy link
Member

mrocklin commented May 6, 2020

Hrm, I tried pushing the following commit to your branch but wasn't able to. Odd.

diff --git a/distributed/cli/tests/test_dask_scheduler.py b/distributed/cli/tests/test_dask_scheduler.py
index d5c0d9c7..fc7469ec 100644
--- a/distributed/cli/tests/test_dask_scheduler.py
+++ b/distributed/cli/tests/test_dask_scheduler.py
@@ -305,6 +305,11 @@ def test_preload_remote_module(loop, tmp_path):
         f.write(PRELOAD_TEXT)
 
     with popen([sys.executable, "-m", "http.server", "9382"], cwd=tmp_path):
+        import requests
+
+        data = requests.get("http://localhost:9382/scheduler_info.py").content
+        assert b"scheduler.foo" in data
+
         with popen(
             [
                 "dask-scheduler",

@jrbourbeau
Copy link
Member Author

GitHub appears to have updated the default setting that allows maintainers to push to other's PRs

Screen Shot 2020-05-05 at 8 32 06 PM

I've applied the diff you posted and checked the box so you should be able to push to this branch now

@mrocklin
Copy link
Member

mrocklin commented May 6, 2020

OK, I broke open the Windows partition. The issue was this #3777

Client.run was running before the preload had a chance to finish. I've resolved the test short term with jrbourbeau#1

@mrocklin
Copy link
Member

mrocklin commented May 6, 2020

There is a nicer solution in there now

@jrbourbeau
Copy link
Member Author

Thanks for fixing this @mrocklin!

@galipremsagar
Copy link

Could this PR be merged? - Asking for other PR's which are blocked by CI failures

@mrocklin mrocklin merged commit 3e47fa0 into dask:master May 6, 2020
@galipremsagar
Copy link

Thanks @mrocklin !! 🎉

@mrocklin
Copy link
Member

mrocklin commented May 6, 2020

Thanks for the poke :)

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.

3 participants