PagerDuty / pdpyras

Low-level PagerDuty REST/Events API client for Python
MIT License
130 stars 30 forks source link

Support for Events v2 API #3

Closed jjm closed 5 years ago

jjm commented 5 years ago

I'm really liking pdpyras for working with the PagerDuty API, however it would be really nice if could support the Events API too. As low level handling rate limits and errors for sending events from custom applications would be really helpful.

Usage would be something like:

from pdpyras import EventSession

integration_key = 'your-token-here'
session = EventSession(integration_key)

event = {}

session.enqueue(event)
Deconstrained commented 5 years ago

Thanks for the input @jjm ! I'm going to start another development cycle on this project that will include support for multi-update endpoints, and I plan on including this one as well.

Deconstrained commented 5 years ago

On second thought - I would like to take some more time to think about how best to incorporate this. This will not likely make it into the next release (coming up shortly), but I do have the intention of working on it.

The library was originally intended for REST API calls only. However, that's not to say that certain features of it (i.e. connection pooling) couldn't also be put to good use in an Events API client.

I'm averse to any quick solution here and would rather like to implement one that will result in the least amount of tech debt. That being said, I think what I'll be doing is refactoring the common features into a base class and creating a new session class for the Events API.

jjm commented 5 years ago

@Deconstrained Thanks for the update, I understand not wanting a quick solution. Look forward to seeing this when you have done some refactoring.

Deconstrained commented 5 years ago

@jjm Yesterday I reviewed my earlier work, ran some final tests and merged/published version 3 of the client, which includes a new class for triggering via the Events API. The new class for utilizing the Events API is called EventsAPISession.

Note, there are some typos in the documentation that I'm working on fixing in addition to some other general improvements.

jjm commented 5 years ago

@Deconstrained Thank you, looks awesome 💯

Deconstrained commented 5 years ago

@jjm Something I just realized would be necessary to avoid superfluous keyword arguments and also better document the trigger and send_event methods would be a lightweight refactor of the args specific to triggering into the trigger method.

I caught this just today going back and noticing a few holes in the documentation (sorry for missing that) and I wanted to run this by you first since you were the original requester, because it makes a change that has the potential to break backwards compatibility. One would be affected by this change only if calling EventsAPISession.send_event directly, and passing in summary and dedup_key as positional arguments rather than keyword arguments as they were intended to be passed.

The changes are in commit e277ece, which I would like to merge in.

Please let me know what you think.

Deconstrained commented 5 years ago

FYI I have merged and am closing this issue again. The follow-up update is a somewhat trivial change and the longer it waits the greater the chances of it having impact.