Added initial support for spot instances

[stumitchell]

Hi I really don't know how the test harness is setup but this code does work in practice,

If I was to improve it I would make sure the try except handling for ec2 errors is around the right code,

and I would issue a warning if there are 0 non-spot instances (actually I haven't tested that so that probably fails too)

Stu

[quasiben]

Thanks for the PR @stumitchell ! I suspect things aren't passing CI because Travis times out. Looking briefly over the code it looks like things probably halt in wait_for_fulfillment(..) . I suspect we can mock some of those things out with moto: https://github.com/spulec/moto/blob/master/tests/test_ec2/test_spot_instances.py

I think we also want defaults to not use spot pricing: spot_count=0.

Because this doesn't require running machine the unittest suite can be fully executed with the following:

py.test dask_ec2/tests -s -vv

or a particular file:

py.test dask_ec2/tests/test_cluster.py -s -vv

or a particular function in a file:

py.test dask_ec2/tests/test_cluster.py::test_from_boto3 -s -vv

[stumitchell]

Looks like master is failing with the same errors I am getting on this pr, so I'll wait till it is fixed

[quasiben]

@stumitchell thanks for looking at CI. Resolving shortly

[quasiben]

@stumitchell finished fixing CI. Rebase against master and push to trigger CI again

[coveralls]

Coverage decreased (-1.05%) to 73.004% when pulling ae76aa1fde7e9ffabf3369e5d2ba906bcd1af7c1 on stumitchell:spot-instances into 9c60d9208ab6ff5e52ffc91227bb0c8b534a760e on dask:master.

[coveralls]

Coverage decreased (-1.05%) to 73.004% when pulling ae76aa1fde7e9ffabf3369e5d2ba906bcd1af7c1 on stumitchell:spot-instances into 9c60d9208ab6ff5e52ffc91227bb0c8b534a760e on dask:master.

[stumitchell]

Right this now passes but the new code is not covered by tests. It should be fairly simple to add tests to this, but I don't really have the time to do it.

Also I think the code would benefit if one of the team looked at it to add coverage, and perhaps check some of the corner cases i.e. --count=0 (if you don't have any non spots) and the placement of try except blocks.

[swamidass]

Any progress on this? It would be nice if this worked.

[mrocklin]

I don't know of anything. If anyone has something a PR would be most welcome.

On Thu, Dec 28, 2017 at 1:30 PM, swamidass [email protected] wrote:

Any progress on this? It would be nice if this worked.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/dask/dask-ec2/pull/66#issuecomment-354360863, or mute the thread https://github.com/notifications/unsubscribe-auth/AASszH6VxmjS-2zjs9yLRofy0g0fdWyjks5tFAiKgaJpZM4Lx6k9 .

[mrocklin]

Ah, my apologies, I see now that this is a PR. It looks like this PR needs help with testing and coverage as mentioned by @stumitchell above. If you have time to help with this @swamidass that would be great

[mrocklin]

In the mean time, I've generally been recommending Kubernetes to people: http://dask.pydata.org/en/latest/setup/kubernetes.html