Conversation
Make sure the the `timeout` parameter is passed to requests before making an HTTP request. By default we use (5,5) which means 5 seconds for connect and read timeouts.
|
LGTM, 5 seconds is a decent default, IIRC the read timeout is not for completion, just for byte gaps. I think it's going to be rarely set, but may be useful in some specific cases. |
To avoid this warning:
```
ckanapi/ckanapi/cli/load.py:307: SyntaxWarning: "\." is an invalid
escape sequence. Such sequences will not work in the
future. Did you mean "\\."? A raw string is also an option.
new_name = re.sub('[0-9\.-]','',name)
```
wardi
reviewed
Feb 20, 2026
| f = requests.get(obj['image_display_url'], stream=True, timeout=REQUEST_TIMEOUT) | ||
| name,ext = obj['image_url'].rsplit('.',1) #reformulate image_url for new site | ||
| new_name = re.sub('[0-9\.-]','',name) | ||
| new_name = re.sub('[0-9.-]','',name) |
Contributor
There was a problem hiding this comment.
This change make the sub do nothing. I think this is what was intended:
Suggested change
| new_name = re.sub('[0-9.-]','',name) | |
| new_name = re.sub(r'[0-9\.-]','',name) |
Contributor
|
How does this interact with someone setting the timeout via Line 60 in c18c623 |
Contributor
|
Setting a new default timeout of 5 seconds would be a breaking change. Calling datastore_search on a 1M row table will take longer than that just because of computing the total row count. The default should either be really long or unset to minimize breaking existing users' code. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Make sure the the
timeoutparameter is passed to requests before making an HTTP request. By default we use (5,5) which means 5 seconds for connect and read timeouts.I added a separate option for the read timeout as it is specially important in the streaming requests we use to load/dump datasets.
@wardi @EricSoroos what do you think of the default value? currently 5 seconds for both