Cleanup Oncalls and add Tests

[cugini-dbx]

Added a unique id to the oncall container which ensures list works with the Collection implemented pagination logic (requires an id).

Removed the overridden show() method which is inconsistent with how show works on other objects (the oncalls endpoint does not support getting a single oncall, as there is no PagerDuty id for them).

Also tested that filtering with oncalls.list() works (for example oncalls.list(schedule_ids=['<id>']).

I also added some tests for the extensions and oncalls objects. I don't think the extensions tests are exhaustive yet, but it's a start. Likely needs more work to ensure create/updating extensions works.

[cugini-dbx]

Hey @balajijegan could you check and see if this change works for your usecase? I tested that this works locally:

p.oncalls.list(schedule_ids=['<id>'])

[balajijegan]

Hey @balajijegan could you check and see if this change works for your usecase? I tested that this works locally:

p.oncalls.list(schedule_ids=['<id>'])

This returns a generator, but when you parse it, it is empty.

[cugini-dbx]

Hey @balajijegan could you check and see if this change works for your usecase? I tested that this works locally:

p.oncalls.list(schedule_ids=['<id>'])

This returns a generator, but when you parse it, it is empty.

Even if you iterate over it? I tested like this:

    for o in pager.oncalls.list(schedule_ids=['XXXXXX']):
        print o.schedule

and did get the expected result.

[balajijegan]

Unfortunately, it is not getting the result for me. This one works though:

balaji:~/code/pygerduty (master)  $ git diff
diff --git a/pygerduty/v2.py b/pygerduty/v2.py
index f25c154..5b555bf 100755
--- a/pygerduty/v2.py
+++ b/pygerduty/v2.py
@@ -186,11 +186,8 @@ class Collection(object):
                 for item in this_paginated_result:
                     # add items to seen_items only if id is present in response
                     # oncalls does not have a id in response
-                    if not hasattr(item, 'id'):
-                        continue
-                    if item.id in seen_items:
-                        continue
-                    seen_items.add(item.id)
+                    if hasattr(item, 'id') and item.id not in seen_items:
+                        seen_items.add(item.id)
                     yield item

[cugini-dbx]

Hmm interesting. Just to check, you're testing against this diff right? The Oncalls object should have an overridden __init__ setting an id on itself.

Are you running python 2 or 3? I realized I've been testing against 2, but perhaps this is py3 specific.

[balajijegan]

I just realized what is going on. I did not see your changes in this commit - https://github.com/dropbox/pygerduty/pull/72/commits/8a5610141f7af082d2fa50e4eee501816b6f8af5. I rebased my master with your changes and things are working as expected.

The only additional change that might be useful is to do a not implemented for calls other than list

Thanks for the fix

[balajijegan]

Hmm interesting. Just to check, you're testing against this diff right? The Oncalls object should have an overridden __init__ setting an id on itself.

I missed this comment but I just saw the additional commit on the PR. I did not get a notification other than your comments and I missed on of your comment earlier. All is well now.

[balajijegan]

@cugini-dbx What are the next steps here?

[cugini-dbx]

@balajijegan I'm still planning to cut a new release by the end of the week.

[balajijegan]

thanks @cugini-dbx