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

lint python for clean code dcp-682 #37

Open
wants to merge 23 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
c7bbe80
Merge pull request #35 from ebi-ait/master
amnonkhen Feb 24, 2022
ddad285
dcp-682 clean code lint rules for python
amnonkhen Mar 6, 2022
85d3489
dcp-682 clean code linting for python
amnonkhen Mar 6, 2022
9256564
lint readme
amnonkhen Mar 6, 2022
5c81876
lint readme
amnonkhen Mar 6, 2022
0d88cc2
lint readme
amnonkhen Mar 6, 2022
5846b8e
dcp-682 exclude test from lint aor the time being
amnonkhen Mar 7, 2022
f09b172
remove cohesion lint rule for now
amnonkhen Mar 14, 2022
893c091
Update .pylintrc
amnonkhen Mar 15, 2022
e1ef3e3
docs and branch config for linting dcp-682
amnonkhen Mar 23, 2022
3518c54
gh action branch rules dcp-682
amnonkhen Mar 23, 2022
ad835cd
dcp-682 lintint gh action
amnonkhen Mar 23, 2022
3248e93
dcp-682 extend max file length
amnonkhen Mar 23, 2022
55c3e00
dcp-682 lint for clean code
amnonkhen Mar 24, 2022
4d0e19a
dcp-682 lint for clean code
amnonkhen Mar 24, 2022
082f64f
dcp-682 lint for clean code - docs for linting from IDE
amnonkhen Mar 25, 2022
cf982c3
dcp-682 lint for clean code - smaller linter image
amnonkhen Mar 25, 2022
f69f13f
dcp-682 lint for clean code - smaller linter image
amnonkhen Mar 25, 2022
3fdcb74
Merge remote-tracking branch 'origin/task/lint-after-push-dcp-682' in…
amnonkhen Mar 25, 2022
c1065cd
dcp-682 lint for clean code - test a messed up file
amnonkhen Mar 25, 2022
c39f831
dcp-682 lint for clean code - linter config
amnonkhen Mar 26, 2022
98d8c9c
dcp-682 lint for clean code - linter config
amnonkhen Mar 26, 2022
8ec3ec2
dcp-682 lint for clean code - linter config
amnonkhen Mar 26, 2022
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
47 changes: 47 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
[flake8]
################### FILE PATTERNS ##########################

# Provide a comma-separated list of glob patterns to exclude from checks.
exclude =
.git,
__pycache__,
.pytest_cache,
.mypy_cache,
.venv,
.vscode,
.idea,
test
# Provide a comma-separate list of glob patterns to include for checks.
filename =
*.py


################### LINTING ################################
# Report all errors, even if it is on the same line as a `# NOQA` comment.
disable-noqa = False

# Set the maximum length that any line (with some exceptions) may be.
max-line-length = 120
amnonkhen marked this conversation as resolved.
Show resolved Hide resolved
# Set the maximum allowed McCabe complexity value for a block of code.
max-complexity = 10
# Toggle whether pycodestyle should enforce matching the indentation of the opening bracket’s line.
# incluences E131 and E133
hang-closing = True

ignore =
E128 # continuation line under-indented for visual indent
E133 # closing bracket is missing indentation
E201 # whitespace after '('
E225 # missing whitespace around operator
E231 # missing whitespace after ','
E241 #multiple spaces after ','
E252 #missing whitespace around parameter equals
E261 #at least two spaces before inline comment
E266 #too many leading '#' for block comment
E302 #expected 2 blank lines, found 1
E303 #too many blank line
E501 # line too long
F401 #'io' imported but unused
F841 #local variable is assigned to but never used
W293 #blank line contains whitespace
W391 #blank line at end of file
amnonkhen marked this conversation as resolved.
Show resolved Hide resolved
21 changes: 13 additions & 8 deletions .github/workflows/linter.yml
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# This workflow executes several linters on changed files based on languages used in your code base whenever
# This workflow executes several linters on changed files based on
# languages used in your code base whenever
# you push a code or open a pull request.
#
# You can adjust the behavior by modifying this file.
Expand All @@ -7,12 +8,11 @@
name: Lint Code Base

on:
push:
branches: [ task/lint-after-push-dcp-682 ]
pull_request:
branches: [ task/test-linting ]
workflow_dispatch:
branches: [ task/lint-after-push-dcp-682 ]
branches:
- dev
- master


jobs:
run-lint:
Expand All @@ -21,12 +21,17 @@ jobs:
- name: Checkout code
uses: actions/checkout@v2
with:
# Full git history is needed to get a proper list of changed files within `super-linter`
fetch-depth: 0
amnonkhen marked this conversation as resolved.
Show resolved Hide resolved

- name: Lint Code Base
uses: github/super-linter@v4
uses: github/super-linter/slim@v4
env:
ACTIONS_RUNNER_DEBUG: true
VALIDATE_ALL_CODEBASE: false
DEFAULT_BRANCH: master
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
FILTER_REGEX_INCLUDE: .*.py
VALIDATE_PYTHON_BLACK: false
VALIDATE_PYTHON_MYPY: false
VALIDATE_PYTHON_ISORT: false
LINTER_RULES_PATH: ./
31 changes: 31 additions & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
[GENERAL]
ignore-paths=
.git,
__pycache__,
.pytest_cache,
.mypy_cache,
.venv,
.vscode,
.idea,
test

ignore=*.md

[MESSAGES CONTROL]
disable=all
enable=
R0913
C0302, # too many lines
R0915, # too many statements
R0101, # nested blocks level
R0801 # duplicate code
MightyAx marked this conversation as resolved.
Show resolved Hide resolved

[FORMAT]
max-line-length=120
max-module-lines=150


[DESIGN]
max-args=5
max-statements= 15
max-nested-blocks=3
15 changes: 15 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
// Use IntelliSense to learn about possible attributes.
// Hover to view descriptions of existing attributes.
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{
"name": "Python: Current File",
"type": "python",
"request": "launch",
"program": "${file}",
"console": "integratedTerminal"
}
]
}
14 changes: 14 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"python.testing.unittestArgs": [
"-v",
"-s",
"./test",
"-p",
"test_*.py"
],
"python.testing.pytestEnabled": false,
"python.testing.unittestEnabled": true,
"python.linting.flake8Enabled": true,
"python.linting.pylintEnabled": true,
"python.linting.enabled": true
}
54 changes: 43 additions & 11 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,31 @@
# HCA Ingest Broker

[![Build Status](https://travis-ci.org/HumanCellAtlas/ingest-client.svg?branch=master)](https://travis-ci.org/HumanCellAtlas/ingest-broker)
[![Maintainability](https://api.codeclimate.com/v1/badges/c3cb9256f7e92537fa99/maintainability)](https://codeclimate.com/github/HumanCellAtlas/ingest-broker/maintainability)
[![Test Coverage](https://api.codeclimate.com/v1/badges/c3cb9256f7e92537fa99/test_coverage)](https://codeclimate.com/github/HumanCellAtlas/ingest-broker/test_coverage)
[![Docker Repository on Quay](https://quay.io/repository/humancellatlas/ingest-broker/status "Docker Repository on Quay")](https://quay.io/repository/humancellatlas/ingest-broker)
[![GitHub Super-Linter](https://github.com/ebi-ait/ingest-broker/workflows/Lint%20Code%20Base/badge.svg)](https://github.com/marketplace/actions/super-linter)

# HCA Ingest Broker

Web endpoint for submitting spreadsheets for HCA Ingest and basic admin UI.
Web endpoint for submitting spreadsheets for HCA Ingest and basic admin UI.

To run scripts locally you'll need Python 3.6 and all the dependencies in [requirements.txt](requirements.txt).

```
## Setup

```bash
pip install -r requirements.txt
pip install -r requirements-dev.txt
```

# Web Application
## Configuration

The broker uses a locally running ingest core at `localhost:8000` by default. Connecting to a different one
Is done by setting the `INGEST_API` environment variable.

## Running with Python
## Running with Python

Start the web application with
Start the web application with

```bash
python broker/broker_app.py
Expand All @@ -45,7 +52,7 @@ See the [template](.flaskenv.template).
See more in [flask's docs](https://flask.palletsprojects.com/en/2.0.x/cli/#environment-variables-from-dotenv)

## Running With Docker
Alternatively, you can build and run the app with Docker. To run the web application with Docker for build the Docker image with
Alternatively, you can build and run the app with Docker. To run the web application with Docker for build the Docker image with

```bash
docker build . -t ingest-broker:latest
Expand All @@ -62,14 +69,39 @@ or run against the development Ingest API
docker run -p 5000:5000 -e INGEST_API=http://api.ingest.dev.data.humancellatlas.org ingest-broker:latest
```

The application will be available at http://localhost:5000
The application will be available at <http://localhost:5000>

# Docs
## Docs

see [design docs](doc/)
see [design docs](./doc/readme.md)

## Running unit tests

```bash
nosetests test/unit/
```

## linting

We use pylint and flake8 for linting.
The configuration files in this repo, [.flake8](.flake8), [.pylintrc](.pylintrc), control
the specific rules we use which are mainly about clean code: function length,

complexity, etc.

Pull requests to dev and master branches are protected and expect no new
linting violations. See the [linter.yml](.github/workflows/linter.yml)

### linting locally

vscode
* enable linting in the settings
* report appears in the "problems" section

intellij
* install pylint plugin
* flake8 errors do not appear in the IDE (no plugin for it)
* scan only modified files, to narrow the list of found issues

command line
* run flake8 from the project root
21 changes: 21 additions & 0 deletions broker_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@ def index():
new_ui_url = os.environ.get('INGEST_UI')
if new_ui_url:
return redirect(new_ui_url, code=302)
for i in range(1,100):
for j in range(2,200):
for k in range(1,200):
for l in range(1,200):
if j != k:
print('ok')
for i in range(1,100):
for j in range(2,200):
for k in range(1,200):
for l in range(1,200):
if j != k:
print('ok')


for i in range(1,100):
for j in range(2,200):
for k in range(1,200):
for l in range(1,200):
if j != k:
print('ok')

return app.response_class(
response=json.dumps({'message': "Ingest Broker API is running!"}),
status=HTTPStatus.OK,
Expand Down
8 changes: 6 additions & 2 deletions requirements-dev.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
-r requirements.txt
coverage
nose==1.3.7
flake8
flake8-eradicate
flake8-fixme
flake8-pylint
pylint
xlrd
nose==1.3.7
python-dotenv
xlrd