[ADD] external-request-timeout: It could wait for a long time

Calling external request needs to use timeout in order to avoid waiting for a long time

It could be even worse if you have one or many records locked

then calling a requests it could accumulate a lot of locks and

the workers could be used for more time and the system could be down

waiting for release

Library supported for now:
 - requests
 - urllib
 - http
 - suds

pylint_odoo/checkers/no_modules.py

@@ -144,6 +144,12 @@
         'str-format-used',
         settings.DESC_DFLT
     ),
+    'E%d06' % settings.BASE_NOMODULE_ID: (
+        'Use of external request method `%s` without timeout. '
+        'It could wait for a long time',
+        'external-request-timeout',
+        settings.DESC_DFLT
+    ),
     'C%d01' % settings.BASE_NOMODULE_ID: (
         'One of the following authors must be present in manifest: %s',
         'manifest-required-author',
@@ -415,6 +421,14 @@ class NoModuleChecker(misc.PylintOdooChecker):
         }),
     )
 
+    def visit_module(self, node):
+        """Initialize dictionary to store imported nodes names"""
+        self.imported_names = {}
+
+    def leave_module(self, node):
+        """Clear variables"""
+        self.imported_names = {}
+
     def open(self):
         super(NoModuleChecker, self).open()
         self.config.deprecated_field_parameters = \
@@ -589,6 +603,7 @@ def visit_print(self, node):
                           'translation-contains-variable',
                           'print-used', 'translation-positional-used',
                           'str-format-used', 'context-overridden',
+                          'external-request-timeout',
                           )
     def visit_call(self, node):
         infer_node = utils.safe_infer(node.func)
@@ -750,6 +765,30 @@ def visit_call(self, node):
         if self._check_sql_injection_risky(node):
             self.add_message('sql-injection', node=node)
 
+        # requests calls
+        func_name = self.get_func_name(node.func)
+        request_meth_timeout = [
+            'requests.delete', 'requests.get', 'requests.head',
+            'requests.options', 'requests.patch', 'requests.post',
+            'requests.put', 'requests.request',
+            'suds.client.Client',
+            'urllib.request.urlopen',
+            'http.client.HTTPConnection', 'http.client.HTTPSConnection',
+            'serial.Serial', 'smtplib.SMTP',
+            'odoo.addons.iap.models.iap.jsonrpc',
+        ]
+        lib_func = (self.imported_names.get(func_name) or
+                    "%s.%s" % (self.get_func_lib(node.func), func_name))
+        if lib_func in request_meth_timeout:
+            for argument in misc.join_node_args_kwargs(node):
+                if not isinstance(argument, astroid.Keyword):
+                    continue
+                if argument.arg == 'timeout':
+                    break
+            else:
+                self.add_message(
+                    'external-request-timeout', node=node, args=(lib_func,))
+
     @utils.check_messages(
         'license-allowed', 'manifest-author-string', 'manifest-deprecated-key',
         'manifest-required-author', 'manifest-required-key',
@@ -911,12 +950,23 @@ def visit_functiondef(self, node):
                 node.name not in self.config.no_missing_return:
             self.add_message('missing-return', node=node, args=(node.name))
 
-    @utils.check_messages('openerp-exception-warning')
+    @utils.check_messages('openerp-exception-warning', 'external-request-timeout')
     def visit_importfrom(self, node):
         if node.modname == 'openerp.exceptions':
             for (import_name, import_as_name) in node.names:
                 if import_name == 'Warning' and import_as_name != 'UserError':
                     self.add_message('openerp-exception-warning', node=node)
+        # Fill dict "self.imported_names" to know the source library imported
+        request_libs = [
+            'requests', 'urllib.request', 'suds.client', 'http.client',
+            'serial', 'smtplib', 'odoo.addons.iap.models.iap'
+        ]
+        if node.modname in request_libs:
+            self.imported_names.update({
+                alias or name: "%s.%s" % (node.modname, name)
+                for name, alias in node.names
+                # "import requests" will be detected with "requests.get"
+                if not (name in request_libs and not alias)})
 
     @utils.check_messages('class-camelcase')
     def visit_classdef(self, node):

pylint_odoo/test/main.py

@@ -30,6 +30,7 @@
     'duplicate-po-message-definition': 2,
     'duplicate-xml-fields': 9,
     'duplicate-xml-record-id': 2,
+    'external-request-timeout': 47,
     'file-not-used': 6,
     'incoherent-interpreter-exec-perm': 3,
     'invalid-commit': 4,

pylint_odoo/test_repo/broken_module/models/broken_model.py

@@ -3,6 +3,38 @@
 import psycopg2
 from psycopg2 import sql
 from psycopg2.sql import SQL, Identifier
+import requests
+from requests import (
+    delete, get, head, options, patch, post, put, request)
+from requests import (
+    delete as delete_r, get as get_r, head as head_r,
+    options as options_r, patch as patch_r, post as post_r,
+    put as put_r, request as request_r)
+
+import urllib
+from urllib.request import urlopen
+from urllib.request import urlopen as urlopen_r
+
+import suds.client
+from suds.client import Client
+from suds.client import Client as Client_r
+
+import http.client
+from http.client import HTTPConnection, HTTPSConnection
+from http.client import (
+    HTTPConnection as HTTPConnection_r,
+    HTTPSConnection as HTTPSConnection_r,
+)
+
+import smtplib
+import smtplib as smtplib_r
+from smtplib import SMTP
+from smtplib import SMTP as SMTP_r
+
+import serial
+import serial as serial_r
+from serial import Serial
+from serial import Serial as Serial_r
 
 from openerp import fields, models, _
 from openerp.exceptions import Warning as UserError
@@ -14,6 +46,10 @@
 import openerp.addons.broken_module as broken_module2
 import openerp.addons.broken_module.broken_model as broken_model2
 
+import odoo.addons.iap.models.iap.jsonrpc
+from odoo.addons.iap.models.iap import jsonrpc
+from odoo.addons.iap.models import iap
+
 
 other_field = fields.Char()
 
@@ -537,6 +573,133 @@ def func(self, a):
         length = len(a)
         return length
 
+    def requests_test(self):
+        # requests without timeout
+        requests.delete('http://localhost')
+        requests.get('http://localhost')
+        requests.head('http://localhost')
+        requests.options('http://localhost')
+        requests.patch('http://localhost')
+        requests.post('http://localhost')
+        requests.put('http://localhost')
+        requests.request('call', 'http://localhost')
+
+        delete_r('http://localhost')
+        get_r('http://localhost')
+        head_r('http://localhost')
+        options_r('http://localhost')
+        patch_r('http://localhost')
+        post_r('http://localhost')
+        put_r('http://localhost')
+        request_r('call', 'http://localhost')
+
+        delete('http://localhost')
+        get('http://localhost')
+        head('http://localhost')
+        options('http://localhost')
+        patch('http://localhost')
+        post('http://localhost')
+        put('http://localhost')
+        request('call', 'http://localhost')
+
+        # requests valid cases
+        requests.delete('http://localhost', timeout=10)
+        requests.get('http://localhost', timeout=10)
+        requests.head('http://localhost', timeout=10)
+        requests.options('http://localhost', timeout=10)
+        requests.patch('http://localhost', timeout=10)
+        requests.post('http://localhost', timeout=10)
+        requests.put('http://localhost', timeout=10)
+        requests.request('call', 'http://localhost', timeout=10)
+
+        delete_r('http://localhost', timeout=10)
+        get_r('http://localhost', timeout=10)
+        head_r('http://localhost', timeout=10)
+        options_r('http://localhost', timeout=10)
+        patch_r('http://localhost', timeout=10)
+        post_r('http://localhost', timeout=10)
+        put_r('http://localhost', timeout=10)
+        request_r('call', 'http://localhost', timeout=10)
+
+        delete('http://localhost', timeout=10)
+        get('http://localhost', timeout=10)
+        head('http://localhost', timeout=10)
+        options('http://localhost', timeout=10)
+        patch('http://localhost', timeout=10)
+        post('http://localhost', timeout=10)
+        put('http://localhost', timeout=10)
+        request('call', 'http://localhost', timeout=10)
+
+        # urllib without timeout
+        urllib.request.urlopen('http://localhost')
+        urlopen('http://localhost')
+        urlopen_r('http://localhost')
+
+        # urllib valid cases
+        urllib.request.urlopen('http://localhost', timeout=10)
+        urlopen('http://localhost', timeout=10)
+        urlopen_r('http://localhost', timeout=10)
+
+        # suds without timeout
+        suds.client.Client('http://localhost')
+        Client('http://localhost')
+        Client_r('http://localhost')
+
+        # suds valid cases
+        suds.client.Client('http://localhost', timeout=10)
+        Client('http://localhost', timeout=10)
+        Client_r('http://localhost', timeout=10)
+
+        # http.client without timeout
+        http.client.HTTPConnection('http://localhost')
+        http.client.HTTPSConnection('http://localhost')
+        HTTPConnection('http://localhost')
+        HTTPSConnection('http://localhost')
+        HTTPConnection_r('http://localhost')
+        HTTPSConnection_r('http://localhost')
+
+        # http.client valid cases
+        http.client.HTTPConnection('http://localhost', timeout=10)
+        http.client.HTTPSConnection('http://localhost', timeout=10)
+        HTTPConnection('http://localhost', timeout=10)
+        HTTPSConnection('http://localhost', timeout=10)
+        HTTPConnection_r('http://localhost', timeout=10)
+        HTTPSConnection_r('http://localhost', timeout=10)
+
+        # smtplib without timeout
+        smtplib.SMTP('http://localhost')
+        smtplib_r.SMTP('http://localhost')
+        SMTP('http://localhost')
+        SMTP_r('http://localhost')
+
+        # smtplib valid cases
+        smtplib.SMTP('http://localhost', timeout=10)
+        smtplib_r.SMTP('http://localhost', timeout=10)
+        SMTP('http://localhost', timeout=10)
+        SMTP_r('http://localhost', timeout=10)
+
+        # Serial without timeout
+        serial.Serial('/dev/ttyS1')
+        serial_r.Serial('/dev/ttyS1')
+        Serial('/dev/ttyS1')
+        Serial_r('/dev/ttyS1')
+
+        # serial valid cases
+        serial.Serial('/dev/ttyS1', timeout=10)
+        serial_r.Serial('/dev/ttyS1', timeout=10)
+        Serial('/dev/ttyS1', timeout=10)
+        Serial_r('/dev/ttyS1', timeout=10)
+
+        # odoo.addons.iap without timeout
+        odoo.addons.iap.models.iap.jsonrpc('http://localhost')
+        jsonrpc('http://localhost')
+        iap.jsonrpc('http://localhost')
+
+        # odoo.addons.iap valid cases
+        odoo.addons.iap.models.iap.jsonrpc('http://localhost', timeout=10)
+        jsonrpc('http://localhost', timeout=10)
+        iap.jsonrpc('http://localhost', timeout=10)
+
 
 class NoOdoo(object):
     length = 0