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

Add support for pre-populated Sessions #132

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

minor-fixes
Copy link
Contributor

@minor-fixes minor-fixes commented Aug 5, 2022

This PR allows the caller to inject a requests.Session object that the
Nomad client should use to make all requests, in case the client is used
in an environment where cookies/headers/etc. unrelated to Nomad
communication must be added to each request.


Tested - both examples below with this change:

Something like this in my environment fails, as it redirects to an auth server:
(URLs and other bits changed/redacted below)

import nomad

def main():
    n = nomad.Nomad(host="nomad.corp.example.com", port=443, secure=True, verify=True)
    print("num jobs:", len(n.jobs.get_jobs()))

if __name__ == "__main__":
    main()

prints:

Traceback (most recent call last):
  File "client_test_fail.py", line 8, in <module>
    main()
  File "client_test_fail.py", line 5, in main
    print("num jobs:", len(n.jobs.get_jobs()))
  File "/home/scott/dev/python-nomad/nomad/api/jobs.py", line 84, in get_jobs
    return self.request(method="get", params=params).json()
  File "/usr/lib/python3/dist-packages/requests/models.py", line 897, in json
    return complexjson.loads(self.text, **kwargs)
  File "/usr/lib/python3.8/json/__init__.py", line 357, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python3.8/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python3.8/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

whereas preparing and supplying a session:

import requests
import nomad

def main():
    token = "<redacted>"
    s = requests.session()
    s.cookies.set("Creds", token, domain=".corp.example.com")
    with s:
        n = nomad.Nomad(host="nomad.corp.example.com", port=443, secure=True, verify=True, session=s)
        print("num jobs:", len(n.jobs.get_jobs()))

if __name__ == "__main__":
    main()

prints: num jobs: 20

This PR allows the caller to inject a `requests.Session` object that the
Nomad client should use to make all requests, in case the client is used
in an environment where cookies/headers/etc. unrelated to Nomad
communication must be added to each request.

Tested: not yet
@minor-fixes minor-fixes marked this pull request as ready for review August 5, 2022 04:33
@codecov-commenter
Copy link

Codecov Report

Merging #132 (a0c6907) into master (1cc1077) will decrease coverage by 0.22%.
The diff coverage is 92.78%.

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
- Coverage   90.61%   90.38%   -0.23%     
==========================================
  Files          26       27       +1     
  Lines        1065     1103      +38     
==========================================
+ Hits          965      997      +32     
- Misses        100      106       +6     
Impacted Files Coverage Δ
nomad/api/base.py 96.55% <85.71%> (-1.04%) ⬇️
nomad/api/event.py 86.66% <86.66%> (ø)
nomad/__init__.py 97.89% <100.00%> (-0.34%) ⬇️
nomad/api/__init__.py 100.00% <100.00%> (ø)
nomad/api/exceptions.py 100.00% <100.00%> (ø)
nomad/api/job.py 94.59% <100.00%> (+0.22%) ⬆️
nomad/api/jobs.py 90.00% <100.00%> (+0.41%) ⬆️
nomad/api/agent.py 92.85% <0.00%> (-7.15%) ⬇️
nomad/api/node.py 87.69% <0.00%> (+3.07%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jonathanrcross
Copy link
Collaborator

Hey @minor-fixes, thanks for the PR! The tests are passing, seems like some issue with the secret to the test pypi (even though the secret looks correct) thats causing the failure. I'll have to figure that out to get this out for a release.

@jrxFive
Copy link
Owner

jrxFive commented Oct 10, 2022

Thanks @minor-fixes!

@jrxFive jrxFive merged commit 690add1 into jrxFive:master Oct 10, 2022
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