Skip to content

Conversation

@jrnk
Copy link
Contributor

@jrnk jrnk commented Apr 30, 2020

I noticed stopping an allocation was not yet supported. Hereby.
Thanks for your work!

@codecov-io
Copy link

codecov-io commented May 1, 2020

Codecov Report

Merging #111 into master will decrease coverage by 0.04%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #111      +/-   ##
==========================================
- Coverage   78.89%   78.85%   -0.05%     
==========================================
  Files          26       26              
  Lines        1199     1201       +2     
==========================================
+ Hits          946      947       +1     
- Misses        253      254       +1     
Impacted Files Coverage Δ
nomad/api/allocation.py 96.66% <50.00%> (-3.34%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c758391...d5788ff. Read the comment docs.

@erhlee-bird
Copy link
Collaborator

I downloaded your branch and confirmed that the surface functionality is as I expect.

# Create example.nomad and get the full allocation id.
nomad init
nomad run example.nomad
nomad status -verbose example
import sys

import nomad


client = nomad.Nomad()
print(client.allocation.stop_allocation(sys.argv[1]))
{'EvalID': 'ce40dd1e-f002-552e-2844-1fe15617e077', 'Index': 17}
$ nomad status
Allocations
ID        Node ID   Task Group  Version  Desired  Status    Created  Modified
6024fc3b  a7dae536  cache       0        run      running   10s ago  9s ago
556b560e  a7dae536  cache       0        stop     complete  41s ago  10s ago

I additionally ran the tests locally and aside from a few potentially flaky tests, things look good.
Thank you so much for your contribution!

Copy link
Collaborator

@erhlee-bird erhlee-bird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I made one comment on a stylistic change in the tests file that I think would better fit the existing code.

Copy link
Collaborator

@erhlee-bird erhlee-bird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thank you so much for your contribution.

@erhlee-bird erhlee-bird merged commit a3f26fd into jrxFive:master May 6, 2020
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.

4 participants