[What follows is obviously more general than HookDescription.events, which is just an example]

I took this decision based on the following:

• to use the data returned by PyGithub, the user must know its type
• so HookDescription.events is documented as being a list of strings
• according to the "fail fast" principle, PyGithub checks that it will be able to provide a list of string

The consequence is "if /hooks is broken, then Github.get_hooks is broken".

Of course I could raise a more significant exception... raising an AssertionError is just laziness, but it wouldn't change anything for you; Github.get_hooks would still be broken even if you could catch a specific exception.

Can you please develop the "Be liberal in what you accept" part? What behavior would you propose for PyGithub in that case? I really don't want HookDescription.events to silently return an empty list, or None, because it would lead to bugs in user code later, that would be more difficult to detect.

Thank you for your ideas,

So, "Be liberal in what you accept" is a general protocol design principle, most famous from the IETF. (The full quote is, "Be conservative in what you send and liberal in what you accept.") The idea is that, when you set up a protocol data unit that you'll be sending, you want to be very strict in how you formulate it, to improve the chance that everyone will be able to understand it. On the other hand, when you're receiving a PDU, you want to be sensitive to the possibility that other people may have implemented ambiguous parts of the protocol differently from the way you're expecting, so you want to try hard to decode the PDU, even if it's not strictly what you're expecting.

In this specific case, there is a single hook in the list returned by the local github instance that has a badly formatted "events" field. There are a range of options I could suggest for handling that; the first that occurs to me is to simply drop the single bad element. A second is to incorporate a workaround: if the list has one element which is also a list, unwrap the inner list. A third would be to use some sort of sentinel return value (None isn't the only option; you could make up your own, such as github.BadValue) to indicate that we couldn't parse the field; you could either return that or raise an exception if an attempt is made to access the element.

As an example of the third possibility, you could do something like the following:

class BadValue(Exception):
    def __init__(self, raw):
        super(BadValue, self).__init__("Invalid value received from server")
        self.raw = raw

Then, if a value is an instance of BadValue, you raise it when an access attempt is made; the caller can catch it and look at the raw data, if they are so inclined.

Not bad :)

I will raise the exception when the user accesses the faulty attribute instead of when I create the object containing it.

I will be happy because PyGithub will still not be silent about bad data returned by the API, and you will be happy because you will be able to use all the correct data.

Wait for it in next version, I a week or two.

Thanks,

Done in develop, will be in version 1.20.0.

@klmitch You may want to check github.test.BadAttributes.testIssue195 to see if this behavior is ok for you. I hope so, but if not, do not hesitate to reopen this issue.