Skip to content

Commit f5cfb9b

Browse files
authored
Be a lot more conservative in _github_submodule_required (#708)
This function is used to decide whether we can use github directly to download the code without cloning the repository. That optimization only works if the repository doesn't have submodules. Before this commit, if github returned any status code that wasn't a 2xx or 3xx, we would consider that the repository didn't have any submodule. This is fairly obviously wrong as a transient server error on the github side for example would mean that we'd assume that the repository doesn't have any submodules. We now check for HTTPErrors and only consider that a repository doesn't have any submodule iif the status is 404 otherwise, something went wrong, log it and fallback to the non optimized path. I also kept the bare exception but added some log to it, just in case.
1 parent ec4a5ae commit f5cfb9b

File tree

1 file changed

+9
-2
lines changed

1 file changed

+9
-2
lines changed

src/taskgraph/run-task/fetch-content

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -679,8 +679,15 @@ def _github_submodule_required(repo: str, commit: str):
679679
try:
680680
status_code = urllib.request.urlopen(url).getcode()
681681
return status_code == 200
682-
except:
683-
return False
682+
except urllib.error.HTTPError as e:
683+
if e.status == 404:
684+
return False
685+
# If we get a non 2xx status code that isn't a 404, something has gone horribly wrong on the github side, log it and return True
686+
log("Got {} from github while checking for submodules in {} which was unexpected. Cannot check whether the repo has submodules or not".format(e.status, repo))
687+
return True
688+
except Exception as e:
689+
log("Got an unexpected `{}` exception while checking for submodules in {}. Cannot check whether the repo has submodules or not".format(e, repo))
690+
return True
684691

685692

686693
def git_checkout_archive(

0 commit comments

Comments
 (0)