Skip to content
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

Blackify some files to show style differences #1186

Merged
merged 7 commits into from
Sep 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions .flake8

This file was deleted.

1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ wheels/
*.egg-info/
.installed.cfg
*.egg
pip-wheel-metadata/

# PyInstaller
# Usually these files are written by a python script from a template
Expand Down
7 changes: 7 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
repos:
- repo: https://github.com/python/black
rev: 19.3b0
hooks:
- id: black
language_version: python3.7
exclude_types: ['markdown', 'ini', 'toml']
19 changes: 16 additions & 3 deletions DEVELOPMENT_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,20 @@ easily setup multiple Python versions.
5. Make Python versions available in the project:
`pyenv local 3.6.8 2.7.14 3.7.2`

### 2. Activate Virtualenv
### 2. Install Additional Tooling
#### Black
We format our code using [Black](https://github.com/python/black) and verify the source code is black compliant
in Appveyor during PRs. You can find installation instructions on [Black's docs](https://black.readthedocs.io/en/stable/installation_and_usage.html).

After installing, you can run our formatting through our Makefile by `make black-format` or integrating Black directly in your favorite IDE (instructions
can be found [here](https://black.readthedocs.io/en/stable/editor_integration.html))

#### 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](https://pre-commit.com/).
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.

### 3. Activate Virtualenv

Virtualenv allows you to install required libraries outside of the
Python installation. A good practice is to setup a different virtualenv
Expand All @@ -49,7 +62,7 @@ be the appropriate python version.
1. `pyenv virtualenv 3.7.2 samcli37`
2. `pyenv activate samcli37` for Python3.7

### 3. Install dev version of SAM CLI
### 4. Install dev version of SAM CLI

We will install a development version of SAM CLI from source into the
virtualenv for you to try out the CLI as you make changes. We will
Expand All @@ -60,7 +73,7 @@ SAM CLI installation, if any.
2. Install dev CLI: `make init`
3. Make sure installation succeeded: `which samdev`

### 4. (Optional) Install development version of SAM Transformer
### 5. (Optional) Install development version of SAM Transformer

If you want to run the latest version of [SAM
Transformer](https://github.com/awslabs/serverless-application-model/),
Expand Down
12 changes: 5 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,15 @@ smoke-test:
# Smoke tests run in parallel
pytest -n 4 tests/functional

flake:
# Make sure code conforms to PEP8 standards
flake8 samcli
flake8 tests/unit tests/integration

lint:
# Linter performs static analysis to catch latent bugs
pylint --rcfile .pylintrc samcli

# Command to run everytime you make changes to verify everything works
dev: flake lint test
dev: lint test

black:
black samcli/* tests/*

# Verifications to run before sending a pull request
pr: init dev
pr: init dev
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't black run as part of make pr?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the make black command requires you to commit the changes. So I didn't want the make pr to run that and make it look like it works. I can add another make command that does the check instead, which is probably what we want anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you install the git hooks, running the check on pr is just redundant. This is because you can't push unless black passes.

8 changes: 6 additions & 2 deletions appveyor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ for:
- sh: "sudo unzip -d /opt/gradle /tmp/gradle-*.zip"
- sh: "PATH=/opt/gradle/gradle-5.5/bin:$PATH"

# Install black
- sh: "wget -O /tmp/black https://github.com/python/black/releases/download/19.3b0/black"
- sh: "chmod +x /tmp/black"
- sh: "/tmp/black --version"

-
matrix:
only:
Expand All @@ -72,12 +77,11 @@ build_script:

test_script:
- "pytest --cov samcli --cov-report term-missing --cov-fail-under 95 tests/unit"
- "flake8 samcli"
- "flake8 tests/unit tests/integration"
- "pylint --rcfile .pylintrc samcli"

# Runs only in Linux
- sh: "pytest -vv tests/integration"
- sh: "/tmp/black --check setup.py tests samcli"

# Smoke tests run in parallel - it runs on both Linux & Windows
# Presence of the RUN_SMOKE envvar will run the smoke tests
Expand Down
22 changes: 22 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[build-system]
requires = ["setuptools", "wheel"] # PEP 508 specifications.


[tool.black]
line-length = 120
target_version = ['py37', 'py27', 'py36']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove py27?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept it because we still have py27 support :(. So wasn't comfortable making formatting related this and risk breaking it. I just quickly ran it and the only changes were removing u from strings like u"help". For now, I would prefer to just leave it.

exclude = '''
(
/(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also exclude init template directories?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea. sure can do.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have excluded the templates used for sam init. I will un-format them once, I do the update from develop and rerun black dance.

\.eggs # exclude a few common directories in the
| \.git # root of the project
| \.tox
| \.venv
| build
| dist
| pip-wheel-metadata
| samcli/local/init/templates
)/
)
'''
1 change: 0 additions & 1 deletion requirements/dev.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
coverage==4.3.4
flake8==3.3.0
tox==2.2.1
pytest-cov==2.4.0
# astroid > 2.0.4 is not compatible with pylint1.7
Expand Down
2 changes: 1 addition & 1 deletion samcli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
SAM CLI version
"""

__version__ = '0.21.0'
__version__ = "0.21.0"
4 changes: 2 additions & 2 deletions samcli/cli/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"samcli.commands.package",
"samcli.commands.deploy",
"samcli.commands.logs",
"samcli.commands.publish"
"samcli.commands.publish",
]

DEPRECATION_NOTICE = (
Expand Down Expand Up @@ -85,7 +85,7 @@ def _set_commands(package_names):
commands = OrderedDict()

for pkg_name in package_names:
cmd_name = pkg_name.split('.')[-1]
cmd_name = pkg_name.split(".")[-1]
commands[cmd_name] = pkg_name

return commands
Expand Down
7 changes: 3 additions & 4 deletions samcli/cli/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def debug(self, value):

if self._debug:
# Turn on debug logging
logging.getLogger('samcli').setLevel(logging.DEBUG)
logging.getLogger('aws_lambda_builders').setLevel(logging.DEBUG)
logging.getLogger("samcli").setLevel(logging.DEBUG)
logging.getLogger("aws_lambda_builders").setLevel(logging.DEBUG)

@property
def region(self):
Expand Down Expand Up @@ -135,5 +135,4 @@ def _refresh_session(self):
the Boto3's session object are read-only. Therefore when Click parses new AWS session related properties (like
region & profile), it will call this method to create a new session with latest values for these properties.
"""
boto3.setup_default_session(region_name=self._aws_region,
profile_name=self._aws_profile)
boto3.setup_default_session(region_name=self._aws_region, profile_name=self._aws_profile)
6 changes: 3 additions & 3 deletions samcli/cli/global_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def config_dir(self):
if not self._config_dir:
# Internal Environment variable to customize SAM CLI App Dir. Currently used only by integ tests.
app_dir = os.getenv("__SAM_CLI_APP_DIR")
self._config_dir = Path(app_dir) if app_dir else Path(click.get_app_dir('AWS SAM', force_posix=True))
self._config_dir = Path(app_dir) if app_dir else Path(click.get_app_dir("AWS SAM", force_posix=True))
return Path(self._config_dir)

@property
Expand Down Expand Up @@ -106,7 +106,7 @@ def telemetry_enabled(self):
# If environment variable is set, its value takes precedence over the value from config file.
env_name = "SAM_CLI_TELEMETRY"
if env_name in os.environ:
return os.getenv(env_name) in ('1', 1)
return os.getenv(env_name) in ("1", 1)

try:
self._telemetry_enabled = self._get_value(TELEMETRY_ENABLED_KEY)
Expand Down Expand Up @@ -200,7 +200,7 @@ def _set_json_cfg(self, filepath, key, value, json_body=None):
json_body[key] = value
file_body = json.dumps(json_body, indent=4) + "\n"
try:
with open(str(filepath), 'w') as f:
with open(str(filepath), "w") as f:
f.write(file_body)
except IOError as ex:
LOG.debug("Error writing to {filepath}", exc_info=ex)
Expand Down
12 changes: 5 additions & 7 deletions samcli/cli/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from .global_config import GlobalConfig

LOG = logging.getLogger(__name__)
logging.basicConfig(level=logging.INFO, format='%(asctime)s %(message)s', datefmt='%Y-%m-%d %H:%M:%S')
logging.basicConfig(level=logging.INFO, format="%(asctime)s %(message)s", datefmt="%Y-%m-%d %H:%M:%S")


pass_context = click.make_pass_decorator(Context)
Expand Down Expand Up @@ -47,9 +47,7 @@ def print_info(ctx, param, value):
if not value or ctx.resilient_parsing:
return

click.echo(json.dumps({
"version": __version__
}, indent=2))
click.echo(json.dumps({"version": __version__}, indent=2))

ctx.exit()

Expand Down Expand Up @@ -96,9 +94,9 @@ def cli(ctx):
except (IOError, ValueError) as ex:
LOG.debug("Unable to write telemetry flag", exc_info=ex)

sam_cli_logger = logging.getLogger('samcli')
sam_cli_formatter = logging.Formatter('%(message)s')
lambda_builders_logger = logging.getLogger('aws_lambda_builders')
sam_cli_logger = logging.getLogger("samcli")
sam_cli_formatter = logging.Formatter("%(message)s")
lambda_builders_logger = logging.getLogger("aws_lambda_builders")

SamCliLogger.configure_logger(sam_cli_logger, sam_cli_formatter, logging.INFO)
SamCliLogger.configure_logger(lambda_builders_logger, sam_cli_formatter, logging.INFO)
34 changes: 20 additions & 14 deletions samcli/cli/options.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,20 @@ def debug_option(f):
:param f: Callback Function to be passed to Click
"""

def callback(ctx, param, value):
state = ctx.ensure_object(Context)
state.debug = value
return value

return click.option('--debug',
expose_value=False,
is_flag=True,
envvar="SAM_DEBUG",
help='Turn on debug logging to print debug message generated by SAM CLI.',
callback=callback)(f)
return click.option(
"--debug",
expose_value=False,
is_flag=True,
envvar="SAM_DEBUG",
help="Turn on debug logging to print debug message generated by SAM CLI.",
callback=callback,
)(f)


def region_option(f):
Expand All @@ -33,15 +36,15 @@ def region_option(f):
:param f: Callback Function to be passed to Click
"""

def callback(ctx, param, value):
state = ctx.ensure_object(Context)
state.region = value
return value

return click.option('--region',
expose_value=False,
help='Set the AWS Region of the service (e.g. us-east-1).',
callback=callback)(f)
return click.option(
"--region", expose_value=False, help="Set the AWS Region of the service (e.g. us-east-1).", callback=callback
)(f)


def profile_option(f):
Expand All @@ -50,12 +53,15 @@ def profile_option(f):
:param f: Callback Function to be passed to Click
"""

def callback(ctx, param, value):
state = ctx.ensure_object(Context)
state.profile = value
return value

return click.option('--profile',
expose_value=False,
help='Select a specific profile from your credential file to get AWS credentials.',
callback=callback)(f)
return click.option(
"--profile",
expose_value=False,
help="Select a specific profile from your credential file to get AWS credentials.",
callback=callback,
)(f)
8 changes: 3 additions & 5 deletions samcli/cli/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ class CfnParameterOverridesType(click.ParamType):
__EXAMPLE = "ParameterKey=KeyPairName,ParameterValue=MyKey ParameterKey=InstanceType,ParameterValue=t1.micro"

# Regex that parses CloudFormation parameter key-value pairs: https://regex101.com/r/xqfSjW/2
_pattern = r'(?:ParameterKey=([A-Za-z0-9\"]+),ParameterValue=(\"(?:\\.|[^\"\\]+)*\"|(?:\\.|[^ \"\\]+)+))'
_pattern = r"(?:ParameterKey=([A-Za-z0-9\"]+),ParameterValue=(\"(?:\\.|[^\"\\]+)*\"|(?:\\.|[^ \"\\]+)+))"

name = ''
name = ""

def convert(self, value, param, ctx):
result = {}
Expand All @@ -27,9 +27,7 @@ def convert(self, value, param, ctx):
groups = re.findall(self._pattern, value)
if not groups:
return self.fail(
"{} is not in valid format. It must look something like '{}'".format(value, self.__EXAMPLE),
param,
ctx
"{} is not in valid format. It must look something like '{}'".format(value, self.__EXAMPLE), param, ctx
)

# 'groups' variable is a list of tuples ex: [(key1, value1), (key2, value2)]
Expand Down
Loading