-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add timeout= parameter to all requests.* calls #6408
Conversation
Requests will block forever if the remote server hangs and there's no timeout. * Two new config variables: - ckan.resource_proxy.timeout -- specifically for resourceproxy - ckan.requests.timeout -- for all other uses of requests in the code base. * All timeouts currently defaulting to 10 sec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@EricSoroos This looks great, just a couple minor comments. This looks like can easily be backported so you can just submit one PR against master and we'll cherry-pick the commits to the previous versions
ckan/config/deployment.ini_tmpl
Outdated
@@ -62,6 +62,10 @@ ckan.datastore.default_fts_index_method = gist | |||
ckan.site_url = | |||
#ckan.use_pylons_response_cleanup_middleware = true | |||
|
|||
# Default timeout for Requests | |||
#ckan.requests.timeout = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's lower the default to 5 seconds in this case
ckan/lib/captcha.py
Outdated
|
||
import requests | ||
|
||
TIMEOUT = asint(config.get('ckan.requests.timeout', 10)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than get this value on each module, we can ensure it's always present in the config object by setting it at startup time in this method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you saying that I should add config.set_default('ckan.requests.timeout', 5)
in the update_config method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And how does that interact with the new config_declaration.yaml? Does that actually apply at runtime if there's a default value?
Also required in ckanext-geview: https://github.com/ckan/ckanext-geoview/blob/master/ckanext/geoview/utils.py#L82 |
@EricSoroos do you want to clean this up and address the comments so it can get in the next patch release? |
…ployment.ini_templ
* call get_value * remove asint * default value is handled in config declaration system.
* call get_value * remove asint * default value is handled in config declaration system.
I don't think any of the commits will cherry pick to 2.8 or 2.9. |
Unclear if the test failure is mine. |
Thanks @EricSoroos. I've adapted and cherry-picked this to 2.9 (fd2da1b) and 2.8 (c031752), would be great if you could do a quick review of those. cheers! |
The backports look ok to me. |
I think the "Document new options" commit: 81177ed needs to be merged into master as the tests seem to be failing at the moment...the new |
[#6408] Document new options
* [docs-only] Override click version to avoid conflict * [docs-only] Remove click version from other req files * [docs-only] Remove click override * Remove erroneous argument `--config` isn't a valid option to `rebuild` * Catch TypeError from invalid thrown by dateutils Also provide better logs so that it is possible to identify the issue. * Update search index date tests to throw TypeErrors * Revert "Catch TypeError from invalid dates thrown by dateutils" * Update version for 2.9.4b * [ckan#5597] Fix DataPusher error during resource_update Create local session in the DataPusher submit action and pass that to task_status_update to avoid interferring with the overall resource_update transaction. See ckan#6137 for details * Improve documentation: resource_patch When someone reads "Uploading a new version of a resource file" is not expected that deletes all extras. * [ckan#6113] Fix user email validator when using name as id parameter CKAN 2.9 introduced the `email_is_unique` validator to prevent duplicates. But the current implementation only works when the id param contains the actual UUID not the name. * Update ckan (paster) commands for ckan 2.9.x * Update ckan (paster) commands for ckan 2.9.x * [I18N] [FIX ckan#5586] Don't cache license translations across requests * Licenses are read and cached in the package model * Translate the license in the helper for each request. * Use CKANRequest object implicitly in templates * Test to ensure that we are using CKANRequest in templates * fix test * fix pep8 test... * [ckan#5998] db init error in alembic * ckan#6086 fix sqlalchemy configuration for datastore * render activity timestamps with title= attribute * Fixture for plugin DB migrations * Remove unnecessary imports * Add pending-migrations * Drop extra tables after tests * update argument * Fix py3 security alerts * Remove blogspam link from the documentation footer * [ckan#6162] fix pagination links for custom org types * [ckan#6179] Fix exception when using solr_user and solr_password on Py3 Fixes ckan#6179 Use the base64 module to encode the Solr Basic Auth header to avoid bytes/str exception on Python 3 * [ckan#6179] Encoding definition * [ckan#6181] Fix page render errors when search facets are not defined * fix guard clause on has_more_facets, ckan#6190 * restore Page import so that other modules can refer to it via 'h.Page', ckan#6190 * [ckan#6207] Fix for g.__timer * Remove not accessed user object * Restore user object to _get_action * Remove not accessed variables * [ckan#2317] fix default ordering consuming resource content from datastore * [ckan#2317] added changelog fragement * [ckan#2317] made default sort by _id less aggressive * [ckan#2317] restrict default order to non distinct case * [2317] accepted proposed change in changelog Co-authored-by: Ian Ward <[email protected]> * [ckan#6159] Add and Update Docstring for existing functions * Add doctring to `chained_action` function * Update `chained_auth` docstring * update `chained_helper` docstring * solve conflict * fix pep8 error * update `chained_helper` and `chain_action` doctsting * Fix documentation typo * [ckan#6270] Typo in default_package_sort config example * DataPusher and XLoader documented in Source docs * Update install-from-source.rst * fixes HTTP basic auth cred handling * [ckan#6319] Fix datetime formatting when listing user tokens on py2. * Fix typo * coerce query string keys/values before passing to quote() * ensure fallback behaviour matches default * remove unused import * reversed function used * failing test case for validation error list position * Revert "[ckan#4801] Simplify even more the erros handling in navl" This reverts commit 7b6702b. * Revert "[ckan#4801] Refactor processing of errors in NAVL" This reverts commit a632078. * Revert "[ckan#4801] Refactor processing of errors in NAVL" This reverts commit a632078. * Remove valid lists from error output, add tests * [ckan#6340] Handle Traceback Exception for HTTP and HTTP status Code in logging The traceback for HTTP Exception will be added only when debug is true * [ckan#6340] Add the changelog * [ckan#6340] Coding Standard * [ckan#6340] Change the error level to debug/info * fix for render_datetime * [ckan#6051] Allow UTF-8 in JS translations See issue ckan#6051 * Fix runaway preview height * Fix unpriviledged users being able to access bulk process Any user was able to access and view the bulk process page for organization management. This fix checks access and returns unauthorized exception if theuser shouldn't be there. * Move auth check to _prepare method Previously implementation only worked for GET requests, now works with GET and POST requests * Use bulk_update_public to check access Was using group_update. Also removed duplicate check from POST request * Update tests * fixing access atom feed for a deleted organization * fixing access atom feed for a deleted group * Call to defaul action to allow links to be opened in new tab * Switch to JQuery recommended method * [ckan#6051] Py2/py3 compatible version of open * Remove leftover paste import * Coding standards * Rebuild requirements.txt to sync it with requirements.in * Fix io.open call in lib/i18n.py * Pin watchdog on py3 so it installs a compatible version * Use codecs instead of io * Fix bad merge of ckan#6149 * Rename lib/io.py module which was giving problems * Rename __init__.py file in extension * Show job title on job start/finish log messages To make it easier to debug background job calls. Before: ``` INFO [ckan.lib.jobs] Worker rq:worker:f0792c8bd67344f288b5704d39c43124 starts job 2baa42e5-4582-4103-92e5-b4a384d0b1da from queue "default" ``` After: ``` INFO [ckan.lib.jobs] Worker rq:worker:f0792c8bd67344f288b5704d39c43124 starts job 2baa42e5-4582-4103-92e5-b4a384d0b1da (Process data fields) from queue "default" ``` * Add missing __init__.py file * String literals * snippet names rendered in non-debug mode * Update changelog for 2.9.4 * Build frontend * [i18n] Pull po files from Transifex * [i18n] Compile mo files * Upgrade version for 2.9.4 * Update version for 2.9.5b * Consistent cli behavior * pep8 * Py2 compatible fix for ckan#6135 * [ckan#6390] fix user create/edit email validators * Allow strict types for user/group uploads CKAN 2.9 specific changes when cherry-picking: * Replace f-strings with .format() * Don't use faker / Pillow for tests, as there is no faker fixture in the Python 2 version * Add changelog entry for group image types * Move type verification into upload method * Fix APIToken CLI test * Update docs * Link to config options from changelog * Allow children for select2 * Fix children type * [ckan#6531] Py2/py3 compatible version of open * Add select2 features * Undo change * Replace f-string * Fix standards * [ckan#6530] Add Solr 8 support * Set logging level to error in error mail handler * Add RootPathMiddleware to flask stack to support non-root installs running on python 3 * Add previously removed RootPathMiddleware back to common middleware as it is still needed * Added utility functions for common CKAN admin commands. * Use correct auth function when editing organizations * [ckan#5820] fix invite user with existing email error * Fix regression when validating resource subfields (by @TomeCirun) * [ckan#6408] Add timeout param to request get calls (by @EricSoroos) * [ckan#6408] Document new options * Accept empty string in one of validator * Negate empty string check * Fix pep8 * [i18n] Pull translation from Transifex * [i18n] Compile mo files * Compile frontend * Small fix adding virtual env path to ckan command. * Update changelog before 2.9.5 * Include the Solr 8 schema file in the 2.9 branch * Update version for 2.9.5 * Updates to ckan_utils.sh. * move spatial harvester into ckanext-cioos_harvest extension and allow POST requests to the spatial search api endpoint in the spatial api * document spatil harvester config * update submodules * create dev branch in submodules and update * add compile css command * upgrade solr to 8.11.1 * update * update all submodules to latest dev version * update submodules again * update schema * Updated submodule contrib/docker/src/ckanext-cioos_theme * Updated submodule contrib/docker/src/ckanext-spatial * fix a few integration bugs * update * Updated submodule contrib/docker/src/ckanext-cioos_theme * update translations * fix bugs, update logos * Updated submodule contrib/docker/src/ckanext-scheming * add atlantic eov icons * Updated submodule contrib/docker/src/ckanext-cioos_theme * add collapsed option to indicate how truncated fields initially load * add wasRevisionOf to schema.org profile output * release all submodules - merge dev into main * remove geoview pip file from dockerfile Co-authored-by: amercader <[email protected]> Co-authored-by: Chris Wood <[email protected]> Co-authored-by: Ken Tsang <[email protected]> Co-authored-by: Jari Voutilainen <[email protected]> Co-authored-by: Guillermo Ferrer Bosque <[email protected]> Co-authored-by: Carlo Cancellieri <[email protected]> Co-authored-by: Eric Soroos <[email protected]> Co-authored-by: Alexandr Cherniavskyi <[email protected]> Co-authored-by: etj <[email protected]> Co-authored-by: chris48s <[email protected]> Co-authored-by: Sergey Motornyuk <[email protected]> Co-authored-by: robbi5 <[email protected]> Co-authored-by: ThrawnCA <[email protected]> Co-authored-by: pdelboca <[email protected]> Co-authored-by: Christian Buck <[email protected]> Co-authored-by: wrinklenose <[email protected]> Co-authored-by: Ian Ward <[email protected]> Co-authored-by: steveoni <[email protected]> Co-authored-by: Andres Vazquez <[email protected]> Co-authored-by: Gauravp-NEC <[email protected]> Co-authored-by: Erin Doherty <[email protected]> Co-authored-by: Niall McAndrew <[email protected]> Co-authored-by: Sunny-NEC <[email protected]> Co-authored-by: Shubham Mahajan <[email protected]> Co-authored-by: Tome Cirun <[email protected]> Co-authored-by: jescgom <[email protected]> Co-authored-by: franz osorio <[email protected]> Co-authored-by: Dan Mihaila <[email protected]> Co-authored-by: Francesco Frassinelli <[email protected]> Co-authored-by: Teemu Erkkola <[email protected]> Co-authored-by: Jeff Cullis <[email protected]>
* Rename lib/io.py module which was giving problems * Rename __init__.py file in extension * Show job title on job start/finish log messages To make it easier to debug background job calls. Before: ``` INFO [ckan.lib.jobs] Worker rq:worker:f0792c8bd67344f288b5704d39c43124 starts job 2baa42e5-4582-4103-92e5-b4a384d0b1da from queue "default" ``` After: ``` INFO [ckan.lib.jobs] Worker rq:worker:f0792c8bd67344f288b5704d39c43124 starts job 2baa42e5-4582-4103-92e5-b4a384d0b1da (Process data fields) from queue "default" ``` * Add missing __init__.py file * String literals * snippet names rendered in non-debug mode * Update changelog for 2.9.4 * Build frontend * [i18n] Pull po files from Transifex * [i18n] Compile mo files * Upgrade version for 2.9.4 * Update version for 2.9.5b * Consistent cli behavior * pep8 * Py2 compatible fix for ckan#6135 * [ckan#6390] fix user create/edit email validators * Allow strict types for user/group uploads CKAN 2.9 specific changes when cherry-picking: * Replace f-strings with .format() * Don't use faker / Pillow for tests, as there is no faker fixture in the Python 2 version * Add changelog entry for group image types * Move type verification into upload method * Fix APIToken CLI test * Update docs * Link to config options from changelog * Allow children for select2 * Fix children type * [ckan#6531] Py2/py3 compatible version of open * Add select2 features * Undo change * Replace f-string * Fix standards * [ckan#6530] Add Solr 8 support * Set logging level to error in error mail handler * Add RootPathMiddleware to flask stack to support non-root installs running on python 3 * Add previously removed RootPathMiddleware back to common middleware as it is still needed * Added utility functions for common CKAN admin commands. * Use correct auth function when editing organizations * [ckan#5820] fix invite user with existing email error * Fix regression when validating resource subfields (by @TomeCirun) * [ckan#6408] Add timeout param to request get calls (by @EricSoroos) * [ckan#6408] Document new options * Accept empty string in one of validator * Negate empty string check * Fix pep8 * [i18n] Pull translation from Transifex * [i18n] Compile mo files * Compile frontend * Small fix adding virtual env path to ckan command. * Update changelog before 2.9.5 * Include the Solr 8 schema file in the 2.9 branch * Update version for 2.9.5 * Update version for 2.9.6b * Unpin pytz (ckan#6665) * Pytz is a stable package, and should always be at the most recent version * Pin zope.interface to a more recent version (ckan#6665) * Supports py3 > 3.5 * Allows for modern setuptools > 44.1 * fix errno2 * Add Dockerfile.py3 based on d9a49a8 * Check if locale exists on i18n JS API * Add test and changelog * Updates to ckan_utils.sh. * move spatial harvester into ckanext-cioos_harvest extension and allow POST requests to the spatial search api endpoint in the spatial api * document spatil harvester config * update submodules * create dev branch in submodules and update * remove extra comma (cherry picked from commit fb27f2a) * Fixing tests. * --passthrough-errors overrides conflicting options * Describe --passthrough-errors * Add --passthrough-errors example inside docker-compose.yml * Disable reloader when passthrough_errors is set * Add --host 0.0.0.0 to pdb example * add try/except block when creating test data * add compile css command * check for data before attempting to create it again * upgrade solr to 8.11.1 * update * update all submodules to latest dev version * update submodules again * update schema * Updated submodule contrib/docker/src/ckanext-cioos_theme * Updated submodule contrib/docker/src/ckanext-spatial * fix a few integration bugs * update * Updated submodule contrib/docker/src/ckanext-cioos_theme * update translations * fix bugs, update logos * Updated submodule contrib/docker/src/ckanext-scheming * add atlantic eov icons * Updated submodule contrib/docker/src/ckanext-cioos_theme * add collapsed option to indicate how truncated fields initially load * add wasRevisionOf to schema.org profile output * release all submodules - merge dev into main * remove geoview pip file from dockerfile * Merge branch 'cioos' into cioos_dev Removed submodule contrib/docker/src/ckanext-cioos_theme * Fixes a two errors when dealing with a encoded url. * url in question /%EF%AC%81?foo=bar&bz=%AC%81 * This is a unicode character, which can't be decoded from ascii. Jinja templates will handle this if it's unicode, or if it's hex encoded ascii, but can't take a non-unicode string in python 2 and put this in a template. * The querystring was being quoted, which is incorrect, as: 1) the special characters in the query string mean something 2) The rest of the querystring is already quoted. This makes it double quoted, as seen in the datastore file * We don't want to unquote urls before putting them in the template anyway. * There was s further error passing this unicode path to the template resolution, where in posix path, it fails: ``` File '/usr/lib/ckan/default/lib/python2.7/posixpath.py', line 73 in join path += '/' + b UnicodeDecodeError: 'ascii' codec can't decode byte 0xef in position 1: ordinal not in range(128) ``` The solution here is to make sure it's unicode passed into the function. * check for resource * Fix urlparse function call * [ckan#6948] Avoid storing the session on each request Override the `is_null_session()` method on our own custom `BeakerSessionInterface` class to take into account that Beaker always adds two keys to the session. * [ckan#6948] Add tests to ensure we don't create unneeed sessions Move the BeakerSessionInterface class to make testing easier * [ckan#6948] pep8 * [ckan#6948] prefix literal * [ckan#6948] Add more keys for Beaker==1.11.0 (py3) * [ckan#6948] Prefixes * [ckan#6948] Cross-py version compatible fix * [ckan#6948] line too long * [ckan#6948] [ckan#6984] More elegant check, thanks @ThrawnCA * [ckan#6948] pep8 * update * update * Fix additional casting error (str(unicode)->ascii decode error) * add helper csrf_input * replaced package_read with package_show * [ckan#5727] Fix datapusher trigger in case of resource_update without changing URL but using new file via API The notify method in the IResourceUrlChange will be triggered only when the URL is changed or if we do the resource update via the API _submit_to_datapusher will not be triggered and cause the old preview to be displayed. * [ckan#5727] Add the changelog * add organization facets * Expose check_ckan_version to templates * add try/except block when creating test data * check for data before attempting to create it again * Return zero results instead of raising NotFound when vocabulary does not exist * [ckan#5822] Update sqlparse version * reorder resource view button: allow translation * check if dir already exists * lint * remove white space * Exclude site_user from user_list * Remove typing from cherry-pick * [QOL-8368] fix race condition in creating the default site user - creating the user is idempotent so just ignore the error * [ckan#6649] gettext not for metadata fields * [ckan#6743] Include root_path in activity email notifications * [ckan#5857] Extract translations from emails * Improve error when downloading resource * Views return 403 for NotAuthorized * [ckan#6838] Use the headers Reply-to value if its set in the extensions * Fix broken URL in migration docs * ckan_config test mark works with request context * Fix caching logic on logged in users * [ckan#6892] Fix member delete * Fix relative import * Handle missing resources in activity stream * [ckan#6439] Concurrent-safe resource updates * Fix tests after ckan#6820 * lint * Fix tests after ckan#6618 * prefixes * Remove duplicated class * add auth functions for 17 actions that didn't have them before * add bilingual support to resource names * fix formatting to satisfy linter * remove new auth functions from blacklist * add auth function for recently_changed_packages_activity_list * remove recently_changed_packages_activity_list from blacklist * document sitemap generation * add sitemap url to docs * [2.9] Bump markdown requirement to support Python 3.9 * update psycopg2 to support PostgreSQL 12, ckan#5796 * [ckan#6789] Fix error when listing tokens in the CLI in py2 * [ckan#6519] Use get_action in patch actions to allow custom logic * [ckan#6658] Fix not_empty validator to allow falsy values * [ckan#6956] Prevent non-sysadmin users to change their own state * [i18n] Pull translations from Transifex for 2.9.6 * [i18n] Compile mo files * Add user_patch action Needed for the ckan#6956 fix * Fix patch for ckan#6956 * Frontend build * lint * Fix resource file size not updating with resource_patch * Added changelog fragment * [ckan#6817] Fix theme settings * Replace characters in url * Fix url check location * Use user id in auth cookie rather than name * [ckan#6815] Allow get_translated helper to fall back to base version of a language * lint * lint2 * Updated submodule contrib/docker/src/ckanext-cioos_theme * Update CHANGELOG before 2.9.6 * Update version for 2.9.6 * Update version for 2.9.7b * Updated submodule contrib/docker/src/ckanext-spatial * Revert deletion portions of f9084f9 * Restores main_css as an app global * Restores helpsrs.get_rtl_theme * Reset the form after downloading * Perform checks on provided id when creating user * [ckan#7149] Fix organization delete form (via @Zharktas) * Update changelog * Update version for 2.9.7 * fix install docs * add support for multilingual resource description and name * make uri's more visible in the interface * make uri handling more robust and clean up org about page * fix translations * merge in subrepo changes * update sub repos * match eov labels to munged keywords * add missing organization uri display in media grid view * add fq to organization_list api endpoint example query ```/api/3/action/organization_list?q=hakai&all_fields=true&include_extras=true&fq=-organization-uri:code"_ "",&fq=organization-uri:__``` * upgrade postgis to 3.3 * document organization_list fq addition * add matching on org uri during harvest. consolidate uri fields into code field when possible. * update submodules * Updates to functions in ckan_utils.sh. * add harvest object delete chunk instructions * Remove duplicate function, minor edits on ckan_utils.sh. * fix organization matching on UID during ckan harvest * Update the production.ini as variable, and change ec dump/load to generic functions. * Minor fix to ckan_utils.sh. * Updated submodule contrib/docker/src/ckanext-cioos_harvest * update delete harvest objects by chunks code * upgrade ckanext-harvest tp 1.4.1 * adjust get_fully_qualified_package_uri * update submodules * better populate resources during a cioos ckan harvest * allow round brackets in keywords * Updated submodule contrib/docker/src/ckanext-cioos_theme * add tips to instructions doc * [#175] Add or relocate volumes for solr and redis data in docker-compose files (#176) * update pacific css * add WSP logo Co-authored-by: amercader <[email protected]> Co-authored-by: Sergey Motornyuk <[email protected]> Co-authored-by: calexandr <[email protected]> Co-authored-by: Andres Vazquez <[email protected]> Co-authored-by: Francesco Frassinelli <[email protected]> Co-authored-by: Jari Voutilainen <[email protected]> Co-authored-by: Teemu Erkkola <[email protected]> Co-authored-by: Jeff Cullis <[email protected]> Co-authored-by: Eric Soroos <[email protected]> Co-authored-by: cirun <[email protected]> Co-authored-by: Tome Cirun <[email protected]> Co-authored-by: Tomasz Sabała <[email protected]> Co-authored-by: Sergey <[email protected]> Co-authored-by: hq-ods <[email protected]> Co-authored-by: Shubham Mahajan <[email protected]> Co-authored-by: Sunny-NEC <[email protected]> Co-authored-by: Ian Ward <[email protected]> Co-authored-by: Tome Cirun <[email protected]> Co-authored-by: ThrawnCA <[email protected]> Co-authored-by: pdelboca <[email protected]> Co-authored-by: Knud Möller <[email protected]> Co-authored-by: antuarc <[email protected]> Co-authored-by: Konstantin Sivakov <[email protected]> Co-authored-by: Jari Voutilainen <[email protected]> Co-authored-by: I G Borrelli <[email protected]>
Proposed fixes:
Requests will block forever if the remote server hangs and there's no
timeout.
code base.
Features:
Please [X] all the boxes above that apply