added requests session to ApiClient #80

seantibor · 2020-04-19T03:47:28Z

Fixes #62 by adding requests session to ApiClient utility instances. This may be a naive approach, so please let me know if we need additional changes to support this.

added session instance to ApiClient
replaced requests.<method> with self.session.<method> calls
updated mock patches to use util.requests.Session object patches in test_util.py

prschmid · 2020-04-23T11:06:23Z

@seantibor Thank you for your work! We just switched over to using the responses library for better and cleaner testing. I'm going to leave this PR open but, my guess is that when you merge develop in to your branch, this will no longer be relevant.

That being said thank you for your effort in trying to improve this library!

seantibor · 2020-04-25T03:50:38Z

I was able to accept all of the incoming changes from develop, particularly in test_util. The util.ApiClient uses a session object from requests now, which should save some resources and time when making large numbers of requests sequentially.

prschmid

Thanks for taking the time to add this in and it's a great start! I added a suggestion that would make this a bit more flexible and retain backwards compatibility for those who need it.

Could I also trouble you to write some tests for this? Thank you!

prschmid · 2020-05-06T11:18:33Z

zoomus/util.py

@@ -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()


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.

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.

Even better! Great idea!

…ture/requests-session

seantibor · 2020-05-08T19:30:42Z

Ok -- this should be working now. I've implemented the context manager methods, added tests for the context manager and methods, but I have not added tests for the v2 client yet.

Do we want to write V2 client context manager tests too? I've added a lot of tests for the context manager code so I'm not sure if the value is there. Or we could move them to v2 client calls?

tarkatronic · 2020-05-08T20:13:08Z

zoomus/util.py

@@ -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


This is a little bit alarming and confusing to see without proper context. I think this probably warrants an explanatory comment, or maybe self.session could become a property instead? Along the lines of...

@property def session(self): """Provide a session if one is in use; otherwise, provide the requests module directly.""" if self._session is not None: return self._session return requests @session.setter def session(self, val): self._session = val @session.deleter def session(self): self._session = None

REALLY fair point. I'll include your suggestions now. This approach seems a lot more sane

seantibor · 2020-05-08T20:51:08Z

Updated session approach to use setter, getter, and deleter property methods.

…elop

mfldavidson · 2021-03-17T14:30:47Z

Tests passed for all supported Python versions. Would like @prschmid to take a look before confirming merge.

prschmid

Thank you for taking this on and for your patience in getting this approved!!! Just ran the tests locally and it looks like some tests are failing -- based on the error that I'm seeing, if you merge in the latest develop to your branch, those tests should fix themselves.

(~/Development/zoomus:[feature/requests-session]) # nosetests .      
....................................................................................................................................................................EEEEEEEEEEEE.EEE.....................................................................................S.S......
======================================================================
ERROR: test_api_version_defaults_to_2 (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 91, in test_api_version_defaults_to_2
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_api_key (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 95, in test_can_get_api_key
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_api_secret (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 104, in test_can_get_api_secret
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_meeting_component (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 113, in test_can_get_meeting_component
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_phone_component (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 135, in test_can_get_phone_component
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_recording_component (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 129, in test_can_get_recording_component
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_report_component (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 117, in test_can_get_report_component
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_user_component (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 121, in test_can_get_user_component
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_get_webinar_component (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 125, in test_can_get_webinar_component
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_set_api_key (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 99, in test_can_set_api_key
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_set_api_secret (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 108, in test_can_set_api_secret
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_can_use_client_with_context (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 139, in test_can_use_client_with_context
    with ZoomClient("KEY", "SECRET") as client:
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_init_creates_all_components (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 48, in test_init_creates_all_components
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_init_sets_config (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 20, in test_init_sets_config
    client = ZoomClient("KEY", "SECRET")
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

======================================================================
ERROR: test_init_sets_config_with_timeout (tests.zoomus.test_client.ZoomClientTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/prschmid/Development/zoomus/tests/zoomus/test_client.py", line 37, in test_init_sets_config_with_timeout
    client = ZoomClient("KEY", "SECRET", timeout=500)
  File "/Users/prschmid/Development/zoomus/zoomus/client.py", line 64, in __init__
    "token": util.generate_jwt(api_key, api_secret),
  File "/Users/prschmid/Development/zoomus/zoomus/util.py", line 286, in generate_jwt
    return token.decode("utf-8")
AttributeError: 'str' object has no attribute 'decode'

Name                                Stmts   Miss Branch BrPart  Cover
---------------------------------------------------------------------
zoomus/__init__.py                      5      0      0      0   100%
zoomus/client.py                       46      0      2      0   100%
zoomus/components/__init__.py           2      0      0      0   100%
zoomus/components/base.py              13      0      4      0   100%
zoomus/components/meeting.py           48      0     10      0   100%
zoomus/components/metric.py            22      0      0      0   100%
zoomus/components/past_meeting.py      16      0      0      0   100%
zoomus/components/phone.py             15      0      0      0   100%
zoomus/components/recording.py         34      0      8      0   100%
zoomus/components/report.py            39      0      0      0   100%
zoomus/components/user.py              39      0      0      0   100%
zoomus/components/webinar.py           66      0     10      0   100%
zoomus/util.py                        108      1     40      2    98%
---------------------------------------------------------------------
TOTAL                                 453      1     74      2    99%
----------------------------------------------------------------------
Ran 274 tests in 0.751s

FAILED (SKIP=2, errors=15)

Also, looks like the files don't conform to the black formatting guidelines. Can I trouble you to also make sure to run black . when you re-submit your PR? Thanks!

(~/Development/zoomus:[feature/requests-session]) # black . --check
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/phone/test_call_logs.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/phone/test_calling_plans.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/phone/test_numbers_list.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/phone/test_users.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/phone/test_numbers_get.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/past_meeting/test_participants.py
would reformat /Users/prschmid/Development/zoomus/zoomus/components/metric.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/meeting/test_update.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/meeting/test_create.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/components/webinar/test_register.py
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/test_util.py
Oh no! 💥 💔 💥
11 files would be reformatted, 57 files would be left unchanged.

…elop

seantibor · 2021-04-29T15:47:12Z

Ok -- I think I got it. I've run black on everything and merged develop. Tests are running against 3.6.13 and passing. Let me know if you need anything else.

prschmid · 2021-05-05T12:24:15Z

@seantibor Thanks for fixing up those tests! I just verified and they indeed do pass now. I did notice, however, that there still appears to be one file that is in need of formatting

(~/Development/zoomus:[feature/requests-session]) # black . --check
would reformat /Users/prschmid/Development/zoomus/tests/zoomus/test_util.py
Oh no! 💥 💔 💥
1 file would be reformatted, 95 files would be left unchanged.

Assuming you are running the same version of black as the repo (black==20.8b0) you should be able to just do black . and it will automatically format your files correctly.

Thanks for dealing with these formatting details!

…elop

seantibor · 2021-05-05T20:21:09Z

I'm trying to figure out why black keeps formatting this section differently from the command line vs. the pre-commit hook. Possibly a configuration variable I'm missing?

Any ideas?

tarkatronic · 2021-05-05T20:33:29Z

I'm trying to figure out why black keeps formatting this section differently from the command line vs. the pre-commit hook. Possibly a configuration variable I'm missing?

Any ideas?

Usually when that happens it's due to a black version mismatch. I would be sure to check through and make sure that the versions match up between what you have installed, what's listed in requirements, and what's listed in the pre-commit hook.

seantibor · 2021-05-05T22:18:23Z

Sorry for the many extra commits on this trying to get my pre-commit to work. I finally manually ran black 21.4b and skipped the pre-commit hook that was formatting the test_util.py wrong. I'm not entirely sure why my pre-commit got so messed up but this should bring it back in line.

seantibor added 6 commits April 17, 2020 23:05

added metric component and some tests

e2766af

added more tests for metric component

c004f4a

rollback .gitignore commit

dbe3219

added metric component to client

5ea04a3

corrected client tests for new component

24b865b

added requests session to ApiClient

f5fc052

seantibor added 3 commits April 24, 2020 14:27

Merge branch 'develop' into feature/metric

513b176

updated tests to use responses library

51ebe86

Merge branch 'develop' into feature/requests-session

4628fda

prschmid requested changes May 6, 2020

View reviewed changes

seantibor added 2 commits May 8, 2020 15:09

added context manager to ApiClient and tests

302ae76

Merge branch 'develop' of https://github.com/prschmid/zoomus into fea…

7d35a3f

…ture/requests-session

tarkatronic reviewed May 8, 2020

View reviewed changes

added session getter, setter property methods

87a67a0

seantibor requested a review from prschmid May 8, 2020 20:51

seantibor added 3 commits May 11, 2020 10:08

Merge branch 'feature/metric' into develop

97164af

Merge branch 'develop' of https://github.com/prschmid/zoomus into dev…

d9f67a8

…elop

Merge branch 'develop' into feature/requests-session

3a2db45

prschmid requested a review from mfldavidson February 3, 2021 15:15

prschmid assigned mfldavidson Feb 3, 2021

mfldavidson approved these changes Mar 17, 2021

View reviewed changes

prschmid requested changes Apr 28, 2021

View reviewed changes

seantibor added 2 commits April 29, 2021 11:04

Merge branch 'develop' of https://github.com/prschmid/zoomus into dev…

cc8bf43

…elop

merge develop

8939508

seantibor added 2 commits May 5, 2021 16:02

Merge branch 'develop' of https://github.com/prschmid/zoomus into dev…

6f9b62e

…elop

Merge branch 'develop' into feature/requests-session

9f78bfd

seantibor added 3 commits May 5, 2021 18:02

updated to black 21.4

c28e150

again and again

3e19118

ignoring pre-commit hooks

d0a4538

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added requests session to ApiClient #80

added requests session to ApiClient #80

seantibor commented Apr 19, 2020 •

edited

Loading

prschmid commented Apr 23, 2020

seantibor commented Apr 25, 2020

prschmid left a comment

prschmid May 6, 2020

tarkatronic May 6, 2020

prschmid May 6, 2020

seantibor commented May 8, 2020

tarkatronic May 8, 2020

seantibor May 8, 2020

seantibor commented May 8, 2020

mfldavidson commented Mar 17, 2021

prschmid left a comment

seantibor commented Apr 29, 2021

prschmid commented May 5, 2021

seantibor commented May 5, 2021

tarkatronic commented May 5, 2021

seantibor commented May 5, 2021

added requests session to ApiClient #80

Are you sure you want to change the base?

added requests session to ApiClient #80

Conversation

seantibor commented Apr 19, 2020 • edited Loading

prschmid commented Apr 23, 2020

seantibor commented Apr 25, 2020

prschmid left a comment

Choose a reason for hiding this comment

prschmid May 6, 2020

Choose a reason for hiding this comment

tarkatronic May 6, 2020

Choose a reason for hiding this comment

prschmid May 6, 2020

Choose a reason for hiding this comment

seantibor commented May 8, 2020

tarkatronic May 8, 2020

Choose a reason for hiding this comment

seantibor May 8, 2020

Choose a reason for hiding this comment

seantibor commented May 8, 2020

mfldavidson commented Mar 17, 2021

prschmid left a comment

Choose a reason for hiding this comment

seantibor commented Apr 29, 2021

prschmid commented May 5, 2021

seantibor commented May 5, 2021

tarkatronic commented May 5, 2021

seantibor commented May 5, 2021

seantibor commented Apr 19, 2020 •

edited

Loading