dropbox / pygerduty

A Python library for PagerDuty.
MIT License
163 stars 72 forks source link

Events module #53

Closed khardsonhurley closed 7 years ago

khardsonhurley commented 7 years ago

@gmjosack in this diff you will see the following changes:

Format Changes

V1-> V2 Changes

Changes to come

khardsonhurley commented 7 years ago

Do you know why those Travis checks are failing? I tried to run the tests on my machine and also had issues (for example when I tried to run user_test.py I got an import error, even after running python setup.py install). Am I missing something @gmjosack ?

gmjosack commented 7 years ago

Take a look at https://travis-ci.org/dropbox/pygerduty/jobs/236195360#L170

It's failing because flake8 has errors/warnings

gmjosack commented 7 years ago

The requester.execute_request is still tightly reliant on the Pagerduty class. Ideally the methods in the events module would not need to live on a class at all, let alone inherit from the Pagerduty class which is specific to the REST api. You should be able to pull the opener stuff to live with the request code and pull out the json logic into its own abstraction.

khardsonhurley commented 7 years ago

When you recommended pulling the opener stuff, were you referencing this:

try:
        response = (pagerduty.opener.open(request, timeout=pagerduty.timeout).read().decode("utf-8"))

from the execute_request method? Are you suggesting to move it into the Pagerduty class when you say You should be able to pull the opener stuff to live with the request code?

khardsonhurley commented 7 years ago

Travis is complaining about this:

pygerduty/v2.py:522:17: E128 continuation line under-indented for visual indent

But I believe this was something from the original __init__.py file, would you prefer that I leave it or change to quiet the failure?

gmjosack commented 7 years ago

You just need to add a space on line 522 to align proxies with self. Looks like that changed when you updated the attributes passed to init.