Skip to content

added requests session to ApiClient #80

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.

Sign up for GitHub

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

Open
seantibor wants to merge 22 commits into
base: develop
Choose a base branch
from
Open

added requests session to ApiClient #80

Changes from 2 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e2766af
added metric component and some tests
seantibor Apr 18, 2020
c004f4a
added more tests for metric component
seantibor Apr 18, 2020
dbe3219
rollback .gitignore commit
seantibor Apr 18, 2020
5ea04a3
added metric component to client
seantibor Apr 18, 2020
24b865b
corrected client tests for new component
seantibor Apr 19, 2020
f5fc052
added requests session to ApiClient
seantibor Apr 19, 2020
513b176
Merge branch 'develop' into feature/metric
seantibor Apr 24, 2020
51ebe86
updated tests to use responses library
seantibor Apr 24, 2020
4628fda
Merge branch 'develop' into feature/requests-session
seantibor Apr 24, 2020
302ae76
added context manager to ApiClient and tests
seantibor May 8, 2020
7d35a3f
Merge branch 'develop' of https://github.com/prschmid/zoomus into fea…
seantibor May 8, 2020
87a67a0
added session getter, setter property methods
seantibor May 8, 2020
97164af
Merge branch 'feature/metric' into develop
seantibor May 11, 2020
d9f67a8
Merge branch 'develop' of https://github.com/prschmid/zoomus into dev…
seantibor May 11, 2020
3a2db45
Merge branch 'develop' into feature/requests-session
seantibor May 11, 2020
cc8bf43
Merge branch 'develop' of https://github.com/prschmid/zoomus into dev…
Apr 29, 2021
8939508
merge develop
Apr 29, 2021
6f9b62e
Merge branch 'develop' of https://github.com/prschmid/zoomus into dev…
May 5, 2021
9f78bfd
Merge branch 'develop' into feature/requests-session
May 5, 2021
c28e150
updated to black 21.4
May 5, 2021
3e19118
again and again
May 5, 2021
d0a4538
ignoring pre-commit hooks
May 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions zoomus/util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def __init__(self, base_uri=None, timeout=15, **kwargs):
self.timeout = timeout
for k, v in kwargs.items():
setattr(self, k, v)
self.session = requests.Session()
Copy link
Copy Markdown
Owner

@prschmid prschmid May 6, 2020

Choose a reason for hiding this comment

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

This is a great start! For the sake of flexibility can we make this a configurable option and have the ability to start a new session. I.e. Let's make the method signature

def __init__(self, base_uri=None, timeout=15, use_session=True, **kwargs):

Then we can do something like

self.session = requests.Session() ? use_session : requests

This should then either use the Session or requests depending on what was requested.

We'd then also want to add a new method called something like new_session or something like that so that if someone needs a new session for some reason, they could do that without having to reinstantiate the ApiClient.

Copy link
Copy Markdown
Contributor

@tarkatronic tarkatronic May 6, 2020

Choose a reason for hiding this comment

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

Perhaps it might even be better if this let the client be used as a context manager, rather than having to explicitly say use_session. So then, reusing an example from the readme, a user could do something like:

with ApiClient() as client:
    users = client.user.list().content
    meetings = {user["id"]: client.meeting.list(user_id=user["id"]).content for user in users}

This could be implemented by adding basically

def __enter__(self):
    self.session = requests.Session()

def __exit__(self):
    self.session = None

And the individual get/post, etc, optionally call the client if it's present.

Copy link
Copy Markdown
Owner

@prschmid prschmid May 6, 2020

Choose a reason for hiding this comment

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

Even better! Great idea!


@property
def timeout(self):
Expand Down Expand Up @@ -80,7 +81,7 @@ def get_request(self, endpoint, params=None, headers=None):
"""
if headers is None and self.config.get("version") == API_VERSION_2:
headers = {"Authorization": "Bearer {}".format(self.config.get("token"))}
return requests.get(
return self.session.get(
self.url_for(endpoint), params=params, headers=headers, timeout=self.timeout
)

Expand All @@ -104,7 +105,7 @@ def post_request(
"Authorization": "Bearer {}".format(self.config.get("token")),
"Content-Type": "application/json",
}
return requests.post(
return self.session.post(
self.url_for(endpoint),
params=params,
data=data,
Expand Down Expand Up @@ -133,7 +134,7 @@ def patch_request(
"Authorization": "Bearer {}".format(self.config.get("token")),
"Content-Type": "application/json",
}
return requests.patch(
return self.session.patch(
self.url_for(endpoint),
params=params,
data=data,
Expand All @@ -159,7 +160,7 @@ def delete_request(
data = json.dumps(data)
if headers is None and self.config.get("version") == API_VERSION_2:
headers = {"Authorization": "Bearer {}".format(self.config.get("token"))}
return requests.delete(
return self.session.delete(
self.url_for(endpoint),
params=params,
data=data,
Expand All @@ -183,7 +184,7 @@ def put_request(self, endpoint, params=None, data=None, headers=None, cookies=No
data = json.dumps(data)
if headers is None and self.config.get("version") == API_VERSION_2:
headers = {"Authorization": "Bearer {}".format(self.config.get("token"))}
return requests.put(
return self.session.put(
self.url_for(endpoint),
params=params,
data=data,
Expand Down