Make license attribute in response optional #804

electrofelix · 2018-03-21T16:39:53Z

Github Enterprise instances <= 2.12.7 do not include the license
attribute in responses for repository data. Allow this to be not set
and include a note in the documentation that the corresponding attr may
be set to None.

Partially fixes #794

electrofelix · 2018-03-21T16:42:44Z

I had to manually manipulate this recording to remove the license key, so I need to find a way to avoid needing to do this as otherwise this can't be recreated and is just too kludgey, but hopefully it's a start and once I've ironed out how to test this case better, I can apply it for the other cases I come across as needed.

sigmavirus24 · 2018-03-22T00:23:51Z

There's a lint error here.

I had to manually manipulate this recording to remove the license key

The files in tests/unit/json are not recordings. Mostly their copied directly from the GitHub API documentation. It's okay to manually edit those.

electrofelix · 2018-03-22T09:50:16Z

@sigmavirus24 guess I should have mentioned after the last comment I worked out how to test this via the unit tests and force pushed an update, still just learning where everything is, sorry.

Github Enterprise instances <= 2.12.7 do not include the license attribute in responses for repository data. Allow this to be not set and include a note in the documentation that the corresponding attr may be set to None. Partially fixes sigmavirus24#794

sigmavirus24 · 2018-03-23T02:08:01Z

github3/repos/repo.py

@@ -2447,7 +2455,7 @@ def _update_attributes(self, repo):
        self.has_wiki = repo['has_wiki']
        self.homepage = repo['homepage']
        self.language = repo['language']
-        self.original_license = repo['license']
+        self.original_license = repo.get('license', None)


repo.get() returns None implicitly. Can you share why you specified None explicitly here?

purely to match the code style at

github3.py/github3/repos/repo.py

Lines 2458 to 2460 in aa50e04

self.parent = repo.get('parent', None)

if self.parent is not None:

self.parent = ShortRepository(self.parent, self)

and

github3.py/github3/repos/repo.py

Lines 2463 to 2465 in aa50e04

self.source = repo.get('source', None)

if self.source is not None:

self.source = ShortRepository(self.source, self)

Have no attachment to doing it that way, should I update all 3 calls to use the same approach? Or just this new occurrence?

Might as well update all of them. I don't know what I was thinking. 😆

sigmavirus24 · 2018-03-24T14:03:12Z

In retrospect I'm not that fussed either way about .get(key, None). It can be fixed up some other time.

electrofelix force-pushed the convert-license-attr-optional branch from 6c26894 to ec47b71 Compare March 21, 2018 19:02

electrofelix force-pushed the convert-license-attr-optional branch from ec47b71 to 6b43242 Compare March 22, 2018 09:50

electrofelix force-pushed the convert-license-attr-optional branch from 6b43242 to 08b1488 Compare March 22, 2018 09:51

sigmavirus24 reviewed Mar 23, 2018

View reviewed changes

sigmavirus24 merged commit 4c19805 into sigmavirus24:develop Mar 24, 2018

electrofelix deleted the convert-license-attr-optional branch March 26, 2018 13:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Make license attribute in response optional #804

Make license attribute in response optional #804

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	self.parent = repo.get('parent', None)
	if self.parent is not None:
	self.parent = ShortRepository(self.parent, self)

	self.source = repo.get('source', None)
	if self.source is not None:
	self.source = ShortRepository(self.source, self)

Make license attribute in response optional #804

Make license attribute in response optional #804

Uh oh!

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!