You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by to...@apache.org on 2016/04/23 15:47:14 UTC

[1/2] libcloud git commit: Proof of concept

Repository: libcloud
Updated Branches:
  refs/heads/api_compliance_and_quality_checking_script [created] 1f14cf58c


Proof of concept


Project: http://git-wip-us.apache.org/repos/asf/libcloud/repo
Commit: http://git-wip-us.apache.org/repos/asf/libcloud/commit/67fded3c
Tree: http://git-wip-us.apache.org/repos/asf/libcloud/tree/67fded3c
Diff: http://git-wip-us.apache.org/repos/asf/libcloud/diff/67fded3c

Branch: refs/heads/api_compliance_and_quality_checking_script
Commit: 67fded3c1acc01b87e4f1b214dfbe40bc9b0249d
Parents: 2846da7
Author: John Carr <jo...@unrouted.co.uk>
Authored: Fri Dec 27 17:17:04 2013 +0000
Committer: Tomaz Muraus <to...@tomaz.me>
Committed: Sat Apr 23 14:54:02 2016 +0200

----------------------------------------------------------------------
 contrib/lint.py | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 104 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/libcloud/blob/67fded3c/contrib/lint.py
----------------------------------------------------------------------
diff --git a/contrib/lint.py b/contrib/lint.py
new file mode 100644
index 0000000..02c022b
--- /dev/null
+++ b/contrib/lint.py
@@ -0,0 +1,104 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+libcloud linter
+
+This script checks the libcloud codebase for registered drivers that don't
+comply to libcloud API guidelines.
+"""
+
+import inspect
+import os
+
+import libcloud
+
+import libcloud.compute.providers
+from libcloud.compute.base import NodeDriver
+
+import libcloud.dns.providers
+from libcloud.dns.base import DNSDriver
+
+import libcloud.loadbalancer.providers
+from libcloud.loadbalancer.base import Driver
+
+import libcloud.storage.providers
+from libcloud.storage.base import StorageDriver
+
+
+modules = [
+    (libcloud.compute.providers, NodeDriver),
+    (libcloud.dns.providers, DNSDriver),
+    (libcloud.loadbalancer.providers, Driver),
+    (libcloud.storage.providers, StorageDriver),
+    ]
+
+
+warnings = set()
+
+def warning(obj, warning):
+    source_file = os.path.relpath(inspect.getsourcefile(obj), os.path.dirname(libcloud.__file__))
+    source_line = inspect.getsourcelines(obj)[1]
+    if (source_file, source_line, warning) in warnings:
+        # When the error is actually caused by a mixin or base class we can get dupes...
+        return
+    warnings.add((source_file, source_line, warning))
+    print source_file, source_line, warning
+
+for providers, base, in modules:
+    core_api = {}
+    for name, value in inspect.getmembers(base, inspect.ismethod):
+        if name.startswith("_"):
+            continue
+
+        if name.startswith("ex_"):
+            warning(value, "Core driver shouldn't haveex_ methods")
+            continue
+
+        args = core_api[name] = inspect.getargspec(value)
+
+        for arg in args.args:
+            if arg.startswith("ex_"):
+                warning(value, "Core driver method shouldnt have ex_ arguments")
+
+
+    for driver_id in providers.DRIVERS.keys():
+        driver = providers.get_driver(driver_id)    
+
+        for name, value in inspect.getmembers(driver, inspect.ismethod):
+            if name.startswith("_"):
+                continue
+
+            if not name.startswith("ex_") and not name in core_api:
+                warning(value, "'%s' should be prefixed with ex_ or be private as it is not a core API" % name)
+                continue
+
+
+            # Only validate arguments of core API's
+            if name.startswith("ex_"):
+                continue
+
+            argspec = inspect.getargspec(value)
+
+            core_args = set(core_api[name].args)
+            driver_args = set(argspec.args)
+
+            for missing in core_args - driver_args:
+                warning(value, "Core API function should support arg '%s' but doesn't" % missing)
+
+            for extra in driver_args - core_args:
+                if not extra.startswith("ex_"):
+                    warning(value, "Core API function shouldn't take arg '%s'. Should it be prefixed with ex_?" % extra)
+


[2/2] libcloud git commit: Refactor script so it supports generating reports per driver, etc.

Posted by to...@apache.org.
Refactor script so it supports generating reports per driver, etc.


Project: http://git-wip-us.apache.org/repos/asf/libcloud/repo
Commit: http://git-wip-us.apache.org/repos/asf/libcloud/commit/1f14cf58
Tree: http://git-wip-us.apache.org/repos/asf/libcloud/tree/1f14cf58
Diff: http://git-wip-us.apache.org/repos/asf/libcloud/diff/1f14cf58

Branch: refs/heads/api_compliance_and_quality_checking_script
Commit: 1f14cf58c9fb40bdc418badc6e1db4f98f3d5e4e
Parents: 67fded3
Author: Tomaz Muraus <to...@tomaz.me>
Authored: Sat Apr 23 15:38:53 2016 +0200
Committer: Tomaz Muraus <to...@tomaz.me>
Committed: Sat Apr 23 15:46:51 2016 +0200

----------------------------------------------------------------------
 contrib/lint.py | 196 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 148 insertions(+), 48 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/libcloud/blob/1f14cf58/contrib/lint.py
----------------------------------------------------------------------
diff --git a/contrib/lint.py b/contrib/lint.py
index 02c022b..fce15d7 100644
--- a/contrib/lint.py
+++ b/contrib/lint.py
@@ -14,91 +14,191 @@
 # limitations under the License.
 
 """
-libcloud linter
+Script which checks a driver for for compliance against the base API.
 
-This script checks the libcloud codebase for registered drivers that don't
-comply to libcloud API guidelines.
+Right now it checks for the following things:
+
+1. Driver methods which are not part of the base API need to be prefixed with
+   "ex_"
+2. Additional arguments for the methods which are part of the standard API need
+   to be prefixed with "ex_"
+3. Method signature for the methods which are part of the standard API needs to
+   match the signature of of the standard API (ignoring the extension arguments).
 """
 
-import inspect
 import os
+import argparse
+import hashlib
+import inspect
+
+from collections import defaultdict
 
 import libcloud
 
-import libcloud.compute.providers
+from libcloud.compute.providers import get_driver as get_compute_driver
 from libcloud.compute.base import NodeDriver
 
 import libcloud.dns.providers
 from libcloud.dns.base import DNSDriver
 
 import libcloud.loadbalancer.providers
-from libcloud.loadbalancer.base import Driver
+from libcloud.loadbalancer.base import Driver as LBDriver
 
 import libcloud.storage.providers
 from libcloud.storage.base import StorageDriver
 
 
-modules = [
-    (libcloud.compute.providers, NodeDriver),
-    (libcloud.dns.providers, DNSDriver),
-    (libcloud.loadbalancer.providers, Driver),
-    (libcloud.storage.providers, StorageDriver),
-    ]
+# Maps API to base classes
+API_MAP = {
+    'compute': {
+        'get_driver_func': get_compute_driver,
+        'driver_class': NodeDriver,
+        'methods_specs': []
+    }
+}
+
+# Global object which stores all the warnings so we can avoide duplicates
+WARNINGS_SET = set()
+
+
+def get_hash_for_dict(obj):
+    result = hashlib.md5()
+
+    for key, value in obj.items():
+        result.update('%s-%s' % (key, value))
 
+    result = result.hexdigest()
+    return result
 
-warnings = set()
 
-def warning(obj, warning):
+def get_warning_object(obj, message):
     source_file = os.path.relpath(inspect.getsourcefile(obj), os.path.dirname(libcloud.__file__))
     source_line = inspect.getsourcelines(obj)[1]
-    if (source_file, source_line, warning) in warnings:
+
+    result = {}
+    result['source_file'] = source_file
+    result['source_line'] = source_line
+    result['message'] = message
+
+    dict_hash = get_hash_for_dict(result)
+    if dict_hash in WARNINGS_SET:
         # When the error is actually caused by a mixin or base class we can get dupes...
-        return
-    warnings.add((source_file, source_line, warning))
-    print source_file, source_line, warning
-
-for providers, base, in modules:
-    core_api = {}
-    for name, value in inspect.getmembers(base, inspect.ismethod):
-        if name.startswith("_"):
+        return None
+
+    WARNINGS_SET.add(dict_hash)
+    return result
+
+
+def get_method_list_for_base_apis():
+    """
+    Build a list of methods for all the base APIs.
+    """
+    result = defaultdict(dict)
+
+    for api_name, values in API_MAP.items():
+        driver_class = values['driver_class']
+        base_class = driver_class
+        core_api = {}
+
+        base_class_methods = inspect.getmembers(base_class, inspect.ismethod)
+        for name, method in base_class_methods:
+            # Ignore "private" methods
+            if name.startswith('_'):
+                continue
+
+            if name.startswith('ex_'):
+                #warning(method, 'Core driver shouldn\'t have "ex_" methods')
+                continue
+
+            args = inspect.getargspec(method)
+            core_api[name] = args
+
+            for arg in args.args:
+                if arg.startswith('ex_'):
+                    pass
+                    #warning(method, 'Core driver method shouldnt have ex_ arguments')
+
+        result[api_name] = core_api
+
+    return result
+
+
+def get_warnings_driver_for_module(driver_constant, base_api):
+    get_driver = base_api['get_driver_func']
+    methods_specs = base_api['methods_specs']
+
+    driver = get_driver(driver_constant)
+
+    warnings = []
+    for name, method in inspect.getmembers(driver, inspect.ismethod):
+        # Skip "private" methods
+        if name.startswith('_'):
             continue
 
-        if name.startswith("ex_"):
-            warning(value, "Core driver shouldn't haveex_ methods")
+        # Methods which are not part of the base API need to be prefixed with
+        # "ex_"
+        if not name.startswith('ex_') and name not in methods_specs:
+            message = ('"%s" should be prefixed with ex_ or be private as it is not a core API' % (name))
+            warning = get_warning_object(obj=method, message=message)
+            warnings.append(warning)
             continue
 
-        args = core_api[name] = inspect.getargspec(value)
+        if name not in methods_specs:
+            # Method is not part of the base API
+            continue
 
-        for arg in args.args:
-            if arg.startswith("ex_"):
-                warning(value, "Core driver method shouldnt have ex_ arguments")
+        argspec = inspect.getargspec(method)
 
+        core_args = set(methods_specs[name].args)
+        driver_args = set(argspec.args)
 
-    for driver_id in providers.DRIVERS.keys():
-        driver = providers.get_driver(driver_id)    
+        # TODO: Also check the argument order for the base API
+        missing_args = (core_args - driver_args)
+        for missing in missing_args:
+            message = 'Core API function "%s" should support arg "%s" but doesn\'t' % (name, missing)
+            warning = get_warning_object(obj=method, message=message)
+            warnings.append(warning)
 
-        for name, value in inspect.getmembers(driver, inspect.ismethod):
-            if name.startswith("_"):
-                continue
+        extra_args = (driver_args - core_args)
+        for extra in extra_args:
+            if not extra.startswith('ex_'):
+                message = "Core API function shouldn't take arg '%s'. Should it be prefixed with ex_?" % extra
+                warning = get_warning_object(obj=method, message=message)
+                warnings.append(warning)
 
-            if not name.startswith("ex_") and not name in core_api:
-                warning(value, "'%s' should be prefixed with ex_ or be private as it is not a core API" % name)
-                continue
+    # Filter out empty warning objects (dupes)
+    warnings = [warning for warning in warnings if warning is not None]
+    return warnings
 
 
-            # Only validate arguments of core API's
-            if name.startswith("ex_"):
-                continue
+def generate_report_for_driver(warnings):
+    result = []
+
+    for warning in warnings:
+        line = '%s:%s : %s' % (warning['source_file'], warning['source_line'],
+                               warning['message'])
+        result.append(line)
+
+    result = '\n'.join(result)
+    return result
 
-            argspec = inspect.getargspec(value)
 
-            core_args = set(core_api[name].args)
-            driver_args = set(argspec.args)
+if __name__ == '__main__':
+    parser = argparse.ArgumentParser(description='Compliance and quality check')
+    parser.add_argument('--driver-api', action='store', required=True,
+                        help='API of the driver to check')
+    parser.add_argument('--driver-constant', action='store', required=True,
+                        help='Name of the provider constant to check')
+    args = parser.parse_args()
 
-            for missing in core_args - driver_args:
-                warning(value, "Core API function should support arg '%s' but doesn't" % missing)
+    driver_api = args.driver_api
+    driver_constant = args.driver_constant
 
-            for extra in driver_args - core_args:
-                if not extra.startswith("ex_"):
-                    warning(value, "Core API function shouldn't take arg '%s'. Should it be prefixed with ex_?" % extra)
+    base_methods_map = get_method_list_for_base_apis()
 
+    base_methods = base_methods_map[driver_api]
+    API_MAP[driver_api]['methods_specs'] = base_methods
+    warnings = get_warnings_driver_for_module(driver_constant=driver_constant,
+                                              base_api=API_MAP[driver_api])
+    report = generate_report_for_driver(warnings=warnings)
+    print(report)