Skip to content

Commit

Permalink
Fix github API requests after asset upload; PyGithub#2
Browse files Browse the repository at this point in the history
In the old code the self.__hostname would be overwritten with uploads.github.com
but it could not be correctly re-set to api.github.com after completing the upload
Create a separate connection if hostname or port differ in requestblobandcheck

in the end this became quite a large overhaul, to also make this change generic
for e.g. connecting to status.github.com and all methods
  • Loading branch information
mfonville committed May 21, 2018
1 parent 286272a commit 1d311fe
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 26 deletions.
10 changes: 4 additions & 6 deletions github/MainClass.py
Expand Up @@ -72,6 +72,7 @@
atLeastPython3 = sys.hexversion >= 0x03000000

DEFAULT_BASE_URL = "https://api.github.com"
DEFAULT_STATUS_URL = "https://status.github.com"
# As of 2018-05-17, Github imposes a 10s limit for completion of API requests.
# Thus, the timeout should be slightly > 10s to account for network/front-end
# latency.
Expand Down Expand Up @@ -625,8 +626,7 @@ def get_api_status(self):
"""
headers, attributes = self.__requester.requestJsonAndCheck(
"GET",
"/api/status.json",
cnx="status"
DEFAULT_STATUS_URL + "/api/status.json"
)
return Status.Status(self.__requester, headers, attributes, completed=True)

Expand All @@ -639,8 +639,7 @@ def get_last_api_status_message(self):
"""
headers, attributes = self.__requester.requestJsonAndCheck(
"GET",
"/api/last-message.json",
cnx="status"
DEFAULT_STATUS_URL + "/api/last-message.json"
)
return StatusMessage.StatusMessage(self.__requester, headers, attributes, completed=True)

Expand All @@ -653,8 +652,7 @@ def get_api_status_messages(self):
"""
headers, data = self.__requester.requestJsonAndCheck(
"GET",
"/api/messages.json",
cnx="status"
DEFAULT_STATUS_URL + "/api/messages.json"
)
return [StatusMessage.StatusMessage(self.__requester, headers, attributes, completed=True) for attributes in data]

Expand Down
33 changes: 18 additions & 15 deletions github/Requester.py
Expand Up @@ -254,23 +254,29 @@ def __init__(self, login_or_token, password, base_url, timeout, client_id, clien
self.__apiPreview = api_preview
self.__verify = verify

def requestJsonAndCheck(self, verb, url, parameters=None, headers=None, input=None, cnx=None):
return self.__check(*self.requestJson(verb, url, parameters, headers, input, cnx))
def requestJsonAndCheck(self, verb, url, parameters=None, headers=None, input=None):
return self.__check(*self.requestJson(verb, url, parameters, headers, input, self.__customConnection(url)))

def requestMultipartAndCheck(self, verb, url, parameters=None, headers=None, input=None):
return self.__check(*self.requestMultipart(verb, url, parameters, headers, input))
return self.__check(*self.requestMultipart(verb, url, parameters, headers, input, self.__customConnection(url)))

def requestBlobAndCheck(self, verb, url, parameters=None, headers=None, input=None):
o = urlparse.urlparse(url)
self.__hostname = o.hostname
return self.__check(*self.requestBlob(verb, url, parameters, headers, input))
return self.__check(*self.requestBlob(verb, url, parameters, headers, input, self.__customConnection(url)))

def __check(self, status, responseHeaders, output):
output = self.__structuredFromJson(output)
if status >= 400:
raise self.__createException(status, responseHeaders, output)
return responseHeaders, output

def __customConnection(self, url):
cnx = None
if not url.startswith("/"):
o = urlparse.urlparse(url)
if o.hostname != self.__hostname or o.port != self.__port:
cnx = self.__httpsConnectionClass(o.hostname, o.port)
return cnx

def __createException(self, status, headers, output):
if status == 401 and output.get("message") == "Bad credentials":
cls = GithubException.BadCredentialsException
Expand Down Expand Up @@ -303,7 +309,7 @@ def encode(input):

return self.__requestEncode(cnx, verb, url, parameters, headers, input, encode)

def requestMultipart(self, verb, url, parameters=None, headers=None, input=None):
def requestMultipart(self, verb, url, parameters=None, headers=None, input=None, cnx=None):
def encode(input):
boundary = "----------------------------3c3ba8b523b2"
eol = "\r\n"
Expand All @@ -317,9 +323,9 @@ def encode(input):
encoded_input += "--" + boundary + "--" + eol
return "multipart/form-data; boundary=" + boundary, encoded_input

return self.__requestEncode(None, verb, url, parameters, headers, input, encode)
return self.__requestEncode(cnx, verb, url, parameters, headers, input, encode)

def requestBlob(self, verb, url, parameters={}, headers={}, input=None):
def requestBlob(self, verb, url, parameters={}, headers={}, input=None, cnx=None):
def encode(local_path):
if "Content-Type" in headers:
mime_type = headers["Content-Type"]
Expand All @@ -331,7 +337,7 @@ def encode(local_path):

if input:
headers["Content-Length"] = str(os.path.getsize(input))
return self.__requestEncode(None, verb, url, parameters, headers, input, encode)
return self.__requestEncode(cnx, verb, url, parameters, headers, input, encode)

def __requestEncode(self, cnx, verb, url, parameters, requestHeaders, input, encode):
assert verb in ["HEAD", "GET", "POST", "PATCH", "PUT", "DELETE"]
Expand Down Expand Up @@ -372,9 +378,6 @@ def __requestRaw(self, cnx, verb, url, requestHeaders, input):
original_cnx = cnx
if cnx is None:
cnx = self.__createConnection()
else:
assert cnx == "status"
cnx = self.__httpsConnectionClass("status.github.com", 443)
cnx.request(
verb,
url,
Expand Down Expand Up @@ -413,8 +416,8 @@ def __makeAbsoluteUrl(self, url):
url = self.__prefix + url
else:
o = urlparse.urlparse(url)
assert o.hostname in [self.__hostname, "uploads.github.com"], o.hostname
assert o.path.startswith((self.__prefix, "/api/uploads"))
assert o.hostname in [self.__hostname, "uploads.github.com", "status.github.com"], o.hostname
assert o.path.startswith((self.__prefix, "/api/"))
assert o.port == self.__port
url = o.path
if o.query != "":
Expand Down
Expand Up @@ -265,7 +265,7 @@ None
https
GET
status.github.com
443
None
/api/status.json
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
Expand All @@ -276,7 +276,7 @@ None
https
GET
status.github.com
443
None
/api/last-message.json
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
Expand Down
2 changes: 1 addition & 1 deletion github/tests/ReplayData/Status.testGetLastMessage.txt
@@ -1,7 +1,7 @@
https
GET
status.github.com
443
None
/api/last-message.json
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
Expand Down
2 changes: 1 addition & 1 deletion github/tests/ReplayData/Status.testGetMessages.txt
@@ -1,7 +1,7 @@
https
GET
status.github.com
443
None
/api/messages.json
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
Expand Down
2 changes: 1 addition & 1 deletion github/tests/ReplayData/Status.testGetStatus.txt
@@ -1,7 +1,7 @@
https
GET
status.github.com
443
None
/api/status.json
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
None
Expand Down

0 comments on commit 1d311fe

Please sign in to comment.