jrxFive / python-nomad

Client library Hashicorp Nomad
https://python-nomad.readthedocs.io/en/latest/
MIT License
139 stars 73 forks source link

FIX: using namespaces --> This parameter is not passed to the request #81

Closed pserranoa closed 5 years ago

pserranoa commented 5 years ago

Using python-nomad 1.0.1 if you try to access Nomad with namespaces, you can't get the objects of specific namespace. This pull request solve it.

codecov-io commented 5 years ago

Codecov Report

Merging #81 into master will decrease coverage by 9.77%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #81      +/-   ##
==========================================
- Coverage   91.19%   81.41%   -9.78%     
==========================================
  Files          26       26              
  Lines        1033     1033              
==========================================
- Hits          942      841     -101     
- Misses         91      192     +101
Impacted Files Coverage Δ
nomad/api/base.py 88.88% <100%> (-8.65%) :arrow_down:
nomad/api/deployments.py 48.71% <0%> (-41.03%) :arrow_down:
nomad/api/acl.py 58.33% <0%> (-33.34%) :arrow_down:
nomad/api/node.py 56.92% <0%> (-27.7%) :arrow_down:
nomad/api/deployment.py 68.88% <0%> (-24.45%) :arrow_down:
nomad/api/operator.py 76.47% <0%> (-23.53%) :arrow_down:
nomad/api/client.py 78.4% <0%> (-18.19%) :arrow_down:
nomad/api/exceptions.py 84.61% <0%> (-15.39%) :arrow_down:
nomad/api/job.py 80.28% <0%> (-14.09%) :arrow_down:
... and 4 more

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 ec2c529...62081ca. Read the comment docs.

jrxFive commented 5 years ago

Hey @i4s-pserrano, thanks for the quick issue/fix. Do you have sample snippet of whats trying to be done from the client library; instantiation, request, returned output and possibly what the url should have been?

Mostly so I can add a test for this scenario to make sure we don't regress. Thanks!

pserranoa commented 5 years ago

Hey @jrxFive. I great to help you!

I'm trying to get the deployments over one namespace. The api returns and empty list becase the params doest have the parameter (namespace="admin"). This namespace was instanciated with variables

NOMAD_ADDR=https://my_system NOMAD_NAMESPACE=admin NOMAD_TOKEN=xxxx-xxxx-xxxx-xxxx NOMAD_REGION=eu

If I have time this evening, I will add a test.

regards

pserranoa commented 5 years ago

Hi @jrxFive . I added some test. Please review if you are confortable with this changes. At my last commit, I have resolved the conflict with test_job.py

Regards

jrxFive commented 5 years ago

@i4s-pserrano, still looks like there is a conflict with test_job.py also is there a reason why you deleted example_batch_parameterized.json?

pserranoa commented 5 years ago

It was my fault... recovered you job test file