Manage black version using requirement file (#1748)

aahung · web-flow · commit 6355433d1f0d · 2020-10-23T15:21:07.000-07:00
* chore: Manage black version in dev.txt - config pre-commit - config development guide - config travis Refer to the commit below in aws-sam-cli aws/aws-sam-cli@d725db5fbfc698a9f0c7582 * Format using black 20.8b1 * Opt-out black fron dev.txt for Python 2
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -1,7 +1,9 @@
+# https://pre-commit.com/#repository-local-hooks
 repos:
-  - repo: https://github.com/python/black
-    rev: 19.3b0
-    hooks:
-      - id: black
-        language_version: python3.7
-        exclude_types: ['markdown', 'ini', 'toml', 'rst']
+- repo: local
+  hooks:
+    - id: black
+      name: black
+      entry: black
+      language: system
+      types: [python]
diff --git a/.travis.yml b/.travis.yml
@@ -21,12 +21,6 @@ matrix:
 
 install:
 # Install the code requirements
-- mkdir $HOME/bin-black
-- wget -O $HOME/bin-black/black https://github.com/python/black/releases/download/19.10b0/black
-- chmod +x $HOME/bin-black/black
-- export PATH=$PATH:$HOME/bin-black
-- black --version
-
 - make init
 
 # Install Docs requirements
diff --git a/DEVELOPMENT_GUIDE.rst b/DEVELOPMENT_GUIDE.rst
@@ -28,16 +28,44 @@ Setup Python locally using `pyenv`_
 
 2. Install Additional Tooling
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-1. Black
-~~~~~~~~
+Black
+'''''
 We format our code using `Black`_ and verify the source code is black compliant
-in Appveyor during PRs. You can find installation instructions on `Black's docs`_.
+in Appveyor during PRs. Black will be installed automatically with ``make init``.
 
-After installing, you can check your formatting through our Makefile by running `make black-check`. To automatically update your code to match our formatting, please run `make black`. You can also integrate Black directly in your favorite IDE (instructions
-can be found `here`_)
+After installing, you can run our formatting through our Makefile by
+``make black`` or integrating Black directly in your favorite IDE
+(instructions can be found `here <https://black.readthedocs.io/en/stable/editor_integration.html>`__)
+
+(workaround) Integrating Black directly in your favorite IDE
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Since black is installed in virtualenv, when you follow `this
+instruction <https://black.readthedocs.io/en/stable/editor_integration.html>`__,
+``which black`` might give you this
+
+::
+
+   (samtranslator37) $ where black
+   /Users/<username>/.pyenv/shims/black
+
+However, IDEs such as PyCharm (using FileWatcher) will have a hard time
+invoking ``/Users/<username>/.pyenv/shims/black`` and this will happen:
+
+::
+
+   pyenv: black: command not found
+
+   The `black' command exists in these Python versions:
+     3.7.2/envs/samtranslator37
+     samtranslator37
+
+A simple workaround is to use
+``/Users/<username>/.pyenv/versions/samtranslator37/bin/black`` instead of
+``/Users/<username>/.pyenv/shims/black``.
 
 Pre-commit
-~~~~~~~~~~
+''''''''''
 If you don't wish to manually run black on each pr or install black manually, we have integrated black into git hooks through `pre-commit`_.
 After installing pre-commit, run `pre-commit install` in the root of the project. This will install black for you and run the black formatting on
 commit.
diff --git a/Makefile b/Makefile
@@ -20,6 +20,9 @@ dev: test
 # Verifications to run before sending a pull request
 pr: black-check init dev
 
+# Verifications to run before sending a pull request, skipping black check because black requires Python 3.6+
+pr2.7: init dev
+
 define HELP_MESSAGE
 
 Usage: $ make [TARGETS]
diff --git a/requirements/dev.txt b/requirements/dev.txt
@@ -15,3 +15,6 @@ requests>=2.20.0
 
 # CLI requirements
 docopt>=0.6.2
+
+# formatter
+black==20.8b1; python_version >= '3.6'
diff --git a/samtranslator/model/__init__.py b/samtranslator/model/__init__.py
@@ -355,8 +355,7 @@ def to_cloudformation(self, **kwargs):
 
 
 class SamResourceMacro(ResourceMacro):
-    """ResourceMacro that specifically refers to SAM (AWS::Serverless::*) resources.
-    """
+    """ResourceMacro that specifically refers to SAM (AWS::Serverless::*) resources."""
 
     # SAM resources can provide a list of properties that they expose. These properties usually resolve to
     # CFN resources that this SAM resource generates. This is provided as a map with the following format:
diff --git a/samtranslator/model/api/http_api_generator.py b/samtranslator/model/api/http_api_generator.py
@@ -216,7 +216,8 @@ def _construct_api_domain(self, http_api):
             self.domain["EndpointConfiguration"] = "REGIONAL"
         elif endpoint not in ["REGIONAL"]:
             raise InvalidResourceException(
-                self.logical_id, "EndpointConfiguration for Custom Domains must be one of {}.".format(["REGIONAL"]),
+                self.logical_id,
+                "EndpointConfiguration for Custom Domains must be one of {}.".format(["REGIONAL"]),
             )
         domain_config["EndpointType"] = endpoint
         domain_config["CertificateArn"] = self.domain.get("CertificateArn")
@@ -352,7 +353,8 @@ def _construct_alias_target(self, domain):
             alias_target["DNSName"] = fnGetAtt(self.domain.get("ApiDomainName"), "RegionalDomainName")
         else:
             raise InvalidResourceException(
-                self.logical_id, "Only REGIONAL endpoint is supported on HTTP APIs.",
+                self.logical_id,
+                "Only REGIONAL endpoint is supported on HTTP APIs.",
             )
         return alias_target
 
diff --git a/samtranslator/model/iam.py b/samtranslator/model/iam.py
@@ -23,7 +23,11 @@ def construct_assume_role_policy_for_service_principal(cls, service_principal):
         document = {
             "Version": "2012-10-17",
             "Statement": [
-                {"Action": ["sts:AssumeRole"], "Effect": "Allow", "Principal": {"Service": [service_principal]},}
+                {
+                    "Action": ["sts:AssumeRole"],
+                    "Effect": "Allow",
+                    "Principal": {"Service": [service_principal]},
+                }
             ],
         }
         return document
@@ -43,7 +47,11 @@ def stepfunctions_assume_role_policy(cls):
         document = {
             "Version": "2012-10-17",
             "Statement": [
-                {"Action": ["sts:AssumeRole"], "Effect": "Allow", "Principal": {"Service": ["states.amazonaws.com"]},}
+                {
+                    "Action": ["sts:AssumeRole"],
+                    "Effect": "Allow",
+                    "Principal": {"Service": ["states.amazonaws.com"]},
+                }
             ],
         }
         return document
diff --git a/samtranslator/model/lambda_.py b/samtranslator/model/lambda_.py
@@ -99,8 +99,7 @@ class LambdaEventInvokeConfig(Resource):
 
 
 class LambdaLayerVersion(Resource):
-    """ Lambda layer version resource
-    """
+    """Lambda layer version resource"""
 
     resource_type = "AWS::Lambda::LayerVersion"
     property_types = {
diff --git a/samtranslator/model/sam_resources.py b/samtranslator/model/sam_resources.py
@@ -49,8 +49,7 @@
 
 
 class SamFunction(SamResourceMacro):
-    """SAM function macro.
-    """
+    """SAM function macro."""
 
     resource_type = "AWS::Serverless::Function"
     property_types = {
@@ -697,8 +696,7 @@ def _validate_deployment_preference_and_add_update_policy(
 
 
 class SamApi(SamResourceMacro):
-    """SAM rest API macro.
-    """
+    """SAM rest API macro."""
 
     resource_type = "AWS::Serverless::Api"
     property_types = {
@@ -810,8 +808,7 @@ def to_cloudformation(self, **kwargs):
 
 
 class SamHttpApi(SamResourceMacro):
-    """SAM rest API macro.
-    """
+    """SAM rest API macro."""
 
     resource_type = "AWS::Serverless::HttpApi"
     property_types = {
@@ -876,7 +873,13 @@ def to_cloudformation(self, **kwargs):
             disable_execute_api_endpoint=self.DisableExecuteApiEndpoint,
         )
 
-        (http_api, stage, domain, basepath_mapping, route53,) = api_generator.to_cloudformation()
+        (
+            http_api,
+            stage,
+            domain,
+            basepath_mapping,
+            route53,
+        ) = api_generator.to_cloudformation()
 
         resources.append(http_api)
         if domain:
@@ -894,8 +897,7 @@ def to_cloudformation(self, **kwargs):
 
 
 class SamSimpleTable(SamResourceMacro):
-    """SAM simple table macro.
-    """
+    """SAM simple table macro."""
 
     resource_type = "AWS::Serverless::SimpleTable"
     property_types = {
@@ -954,8 +956,7 @@ def _convert_attribute_type(self, attribute_type):
 
 
 class SamApplication(SamResourceMacro):
-    """SAM application macro.
-    """
+    """SAM application macro."""
 
     APPLICATION_ID_KEY = "ApplicationId"
     SEMANTIC_VERSION_KEY = "SemanticVersion"
@@ -973,14 +974,12 @@ class SamApplication(SamResourceMacro):
     }
 
     def to_cloudformation(self, **kwargs):
-        """Returns the stack with the proper parameters for this application
-        """
+        """Returns the stack with the proper parameters for this application"""
         nested_stack = self._construct_nested_stack()
         return [nested_stack]
 
     def _construct_nested_stack(self):
-        """Constructs a AWS::CloudFormation::Stack resource
-        """
+        """Constructs a AWS::CloudFormation::Stack resource"""
         nested_stack = NestedStack(
             self.logical_id, depends_on=self.depends_on, attributes=self.get_passthrough_resource_attributes()
         )
@@ -994,8 +993,7 @@ def _construct_nested_stack(self):
         return nested_stack
 
     def _get_application_tags(self):
-        """Adds tags to the stack if this resource is using the serverless app repo
-        """
+        """Adds tags to the stack if this resource is using the serverless app repo"""
         application_tags = {}
         if isinstance(self.Location, dict):
             if self.APPLICATION_ID_KEY in self.Location.keys() and self.Location[self.APPLICATION_ID_KEY] is not None:
@@ -1009,8 +1007,7 @@ def _get_application_tags(self):
 
 
 class SamLayerVersion(SamResourceMacro):
-    """ SAM Layer macro
-    """
+    """SAM Layer macro"""
 
     resource_type = "AWS::Serverless::LayerVersion"
     property_types = {
@@ -1108,8 +1105,7 @@ def _get_retention_policy_value(self):
 
 
 class SamStateMachine(SamResourceMacro):
-    """SAM state machine macro.
-    """
+    """SAM state machine macro."""
 
     resource_type = "AWS::Serverless::StateMachine"
     property_types = {
@@ -1125,7 +1121,9 @@ class SamStateMachine(SamResourceMacro):
         "Policies": PropertyType(False, one_of(is_str(), list_of(one_of(is_str(), is_type(dict), is_type(dict))))),
         "Tracing": PropertyType(False, is_type(dict)),
     }
-    event_resolver = ResourceTypeResolver(samtranslator.model.stepfunctions.events,)
+    event_resolver = ResourceTypeResolver(
+        samtranslator.model.stepfunctions.events,
+    )
 
     def to_cloudformation(self, **kwargs):
         managed_policy_map = kwargs.get("managed_policy_map", {})
diff --git a/samtranslator/parser/parser.py b/samtranslator/parser/parser.py
@@ -14,7 +14,7 @@ def parse(self, sam_template, parameter_values, sam_plugins):
 
     # private methods
     def _validate(self, sam_template, parameter_values):
-        """ Validates the template and parameter values and raises exceptions if there's an issue
+        """Validates the template and parameter values and raises exceptions if there's an issue
 
         :param dict sam_template: SAM template
         :param dict parameter_values: Dictionary of parameter values provided by the user
diff --git a/samtranslator/plugins/application/serverless_app_plugin.py b/samtranslator/plugins/application/serverless_app_plugin.py
@@ -294,9 +294,11 @@ def on_after_transform_template(self, template):
 
                 # Check each resource to make sure it's active
                 for application_id, template_id in temp:
-                    get_cfn_template = lambda application_id, template_id: self._sar_client.get_cloud_formation_template(
-                        ApplicationId=self._sanitize_sar_str_param(application_id),
-                        TemplateId=self._sanitize_sar_str_param(template_id),
+                    get_cfn_template = (
+                        lambda application_id, template_id: self._sar_client.get_cloud_formation_template(
+                            ApplicationId=self._sanitize_sar_str_param(application_id),
+                            TemplateId=self._sanitize_sar_str_param(template_id),
+                        )
                     )
                     response = self._sar_service_call(get_cfn_template, application_id, application_id, template_id)
                     self._handle_get_cfn_template_response(response, application_id, template_id)
diff --git a/samtranslator/swagger/swagger.py b/samtranslator/swagger/swagger.py
@@ -208,7 +208,13 @@ def add_lambda_integration(
             path_dict[method] = make_conditional(condition, path_dict[method])
 
     def add_state_machine_integration(
-        self, path, method, integration_uri, credentials, request_templates=None, condition=None,
+        self,
+        path,
+        method,
+        integration_uri,
+        credentials,
+        request_templates=None,
+        condition=None,
     ):
         """
         Adds aws APIGW integration to the given path+method.
diff --git a/samtranslator/translator/translator.py b/samtranslator/translator/translator.py
@@ -28,8 +28,7 @@
 
 
 class Translator:
-    """Translates SAM templates into CloudFormation templates
-    """
+    """Translates SAM templates into CloudFormation templates"""
 
     def __init__(self, managed_policy_map, sam_parser, plugins=None):
         """
diff --git a/tests/model/test_api_v2.py b/tests/model/test_api_v2.py
@@ -47,13 +47,16 @@ def test_create_authorizer_fails_with_string_authorization_scopes(self):
                 authorization_scopes="invalid_scope",
             )
         self.assertEqual(
-            e.value.message, "Resource with id [logicalId] is invalid. " + "AuthorizationScopes must be a list.",
+            e.value.message,
+            "Resource with id [logicalId] is invalid. " + "AuthorizationScopes must be a list.",
         )
 
     def test_create_authorizer_fails_with_authorization_scopes_non_oauth2(self):
         with pytest.raises(InvalidResourceException) as e:
             ApiGatewayV2Authorizer(
-                api_logical_id="logicalId", name="authName", authorization_scopes=["scope1", "scope2"],
+                api_logical_id="logicalId",
+                name="authName",
+                authorization_scopes=["scope1", "scope2"],
             )
         self.assertEqual(
             e.value.message,
@@ -67,7 +70,9 @@ def test_create_authorizer_fails_with_authorization_scopes_non_oauth2(self):
     def test_create_authorizer_fails_with_jtw_configuration_non_oauth2(self):
         with pytest.raises(InvalidResourceException) as e:
             ApiGatewayV2Authorizer(
-                api_logical_id="logicalId", name="authName", jwt_configuration={"config": "value"},
+                api_logical_id="logicalId",
+                name="authName",
+                jwt_configuration={"config": "value"},
             )
         self.assertEqual(
             e.value.message,
@@ -78,7 +83,9 @@ def test_create_authorizer_fails_with_jtw_configuration_non_oauth2(self):
     def test_create_authorizer_fails_with_id_source_non_oauth2(self):
         with pytest.raises(InvalidResourceException) as e:
             ApiGatewayV2Authorizer(
-                api_logical_id="logicalId", name="authName", id_source="https://example.com",
+                api_logical_id="logicalId",
+                name="authName",
+                id_source="https://example.com",
             )
         self.assertEqual(
             e.value.message,
@@ -180,7 +187,8 @@ def test_create_jwt_authorizer_no_id_source(self):
     def test_create_lambda_auth_no_function_arn(self):
         with pytest.raises(InvalidResourceException) as e:
             ApiGatewayV2Authorizer(
-                api_logical_id="logicalId", name="lambdaAuth",
+                api_logical_id="logicalId",
+                name="lambdaAuth",
             )
         self.assertEqual(
             e.value.message,
@@ -190,7 +198,9 @@ def test_create_lambda_auth_no_function_arn(self):
     def test_create_lambda_auth_no_authorizer_payload_format_version(self):
         with pytest.raises(InvalidResourceException) as e:
             ApiGatewayV2Authorizer(
-                api_logical_id="logicalId", name="lambdaAuth", function_arn="lambdaArn",
+                api_logical_id="logicalId",
+                name="lambdaAuth",
+                function_arn="lambdaArn",
             )
         self.assertEqual(
             e.value.message,
diff --git a/tests/swagger/test_swagger.py b/tests/swagger/test_swagger.py
diff --git a/tests/test_model.py b/tests/test_model.py
diff --git a/tests/translator/test_function_resources.py b/tests/translator/test_function_resources.py
diff --git a/tox.ini b/tox.ini
