Skip to content

Conversation

@QuLogic
Copy link
Member

@QuLogic QuLogic commented Oct 10, 2016

This is a followup to my comments on #2162. The number of threads does not need to be so high, and if the hostname is invalid, we should not even bother to make the request.

Running 128 parallel HEAD requests is overkill and liable to get someone
banned.
@QuLogic QuLogic added this to the v1.11 milestone Oct 10, 2016
else:
msg = '{} is not a valid resource.'.format(uri)
exceptions.append(ValueError(msg))
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is the continue here for please, I am not convinced it is doing anything active

Copy link
Member Author

@QuLogic QuLogic Oct 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cut/paste leftover; will remove.


# maximum number of threads for multi-threading code
MAXTHREADS = 128
MAXTHREADS = 8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems sensible, given your analysis of run times

try:
result = requests.head(resource)
if (result.status_code == 200 and
resource.startswith('https://scitools.github.io')):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree with this logic change

@marqh
Copy link
Member

marqh commented Oct 10, 2016

neat improvements; thank you @QuLogic

@marqh marqh merged commit a585b27 into SciTools:master Oct 10, 2016
@QuLogic QuLogic deleted the hash-corrections branch October 10, 2016 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants