sigmavirus24 / github3.py

Hi, I'm a library for interacting with GItHub's REST API in a convenient and ergonomic way. I work on Python 3.6+.
https://github3.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.21k stars 404 forks source link

KeyError: 'auto_merge' in v3.0.0 for Github Enterprise #1057

Closed dozhang closed 2 years ago

dozhang commented 3 years ago

We have Github Enterprise installed, when github3.py automaticaly updated to 3.0.0 from 2.0.0, following errors occured:

Traceback (most recent call last): File "/Users/xxxxx/Library/Python/3.9/lib/python/site-packages/github3/models.py", line 44, in __init__ self._update_attributes(json) File "/Users/xxxx/Library/Python/3.9/lib/python/site-packages/github3/pulls.py", line 715, in _update_attributes self.auto_merge = pull["auto_merge"] KeyError: 'auto_merge'

I think the issue is introduced in https://github.com/sigmavirus24/github3.py/pull/1052/files. It sounds like a regression to me, as v2.0.0 works fine. I do not know if it is a github config issue or github version issue, I would suggest to avoid this issue if the attribute is not returned.

tobiashenkel commented 3 years ago

Note that the auto merge feature has been added in Github Enterprise 3.1 where 3.0 is still supported by Github. So I think this should be treated optional.

dozhang commented 3 years ago

It would be great if it can be quickly fixed and released, so that we do not need to pin the github3.py to 2.0.0.

sigmavirus24 commented 3 years ago

This may get fixed but there's no way to test against all supported releases of GHE so it's unlikely this is the only problem and it's even less likely to be released "quickly" because I work on this when it's fun for me.

dozhang commented 3 years ago

github3.py is used in zuul ci (https://zuul-ci.org/), which I assumed is used with many GHE instances. So this issue could potentially affect many other users, too. Maybe we are the first who noticed (or reported) this issue during zuul reinstall in our test env.

sigmavirus24 commented 3 years ago

@dozhang you fail to understand me. I added a lot of the "latest" attributes for many objects in this release. If you're failing with auto_merge it's likely you're going to fail on other things but the API responses aren't clearly defined for any of the enterprise versions without an installation. Even if I fix this one attribute, I don't believe it's likely that you won't have other issues. GHE support is best effort purely because there's barely enough time to keep up with GitHub now that they're actually tending to their API again

sigmavirus24 commented 3 years ago

Also their OpenAPI descriptions are barely useful but also deeply wrong if you look at the issue tracker, so while that purports to document enterprise versions, they're very clearly wrong and not an avenue for understanding what's actually present/supported

dozhang commented 3 years ago

Actually I understood your comment. I can also not tell what else might fail. How ever, in my opinion you can fix what we already know it will fail as "best effort".

sigmavirus24 commented 3 years ago

@dozhang if I fix that, and something else is broken, will you promise not to open another issue? No? Then can you help sort out what is actually supported? You could fork this, edit the code appropriately until it works on GHE 3.0 and then send a PR. I don't have the patience for fixing 1 attribute, releasing, on a loop until you've tested each one and told me which attributes don't exist

As it stands, I've put forth my best effort. I'm not putting forward countless hours of work though without an end in sight

dozhang commented 2 years ago

PR for fixing this issue: https://github.com/sigmavirus24/github3.py/pull/1060

I have tested the method, and the "auto_merge" is the only attribute I have noticed that cause a problem.