Github workflow for linting (flake8, black, isort) and testing

[vegu]

Github workflow for linting (flake8, black, isort) and testing, can look here for an example.

Also fix flake8 issues if there are any. Lets discuss things like "function too complex" prior to implementation changes though.

Failing unit tests can be ignored for the scope of this, we will tackle those separately.

[egfrank]

While we do this, I think it's worthwhile to think through which of these tools we want to take precedence. Flake8 and Black definitely conflict in their default settings: i notice that Grizz commented out Flake8 of the Github actions in ctl.

But I actually noticed also sometimes Isort and Black conflict:

-from fullctl.django.models.concrete import (Instance, Organization,
-                                            )
+from fullctl.django.models.concrete import (
+    Instance,
+    Organization,
+)

I think Isort does the inline version (-) and Black does the list (+).

Just noting it for now, I'm sure there's ways to configure either Isort or Black to play nicely together.

EDIT: Okay there's info in the Isort docs to make it Black compatible https://pycqa.github.io/isort/docs/configuration/black_compatibility/

[egfrank]

Okay, as of now the Github actions are failing on tests, which is where we want it. https://github.com/fullctl/ixctl/runs/1772822310?check_suite_focus=true

That being said, the flake8 check is commented out. So the codebase on branch gh_36 is passing the black --check but wouldn't pass flake8. Here's what running flake on my computer looks like:

src/django_ixctl/apps.py:9:9: F401 'django_ixctl.signals' imported but unused
src/django_ixctl/urls.py:1:1: F401 'django.conf.settings' imported but unused
src/django_ixctl/urls.py:2:1: F401 'django.contrib.auth.views as auth_views' imported but unused
src/django_ixctl/urls.py:4:1: F401 'django.views.generic.TemplateView' imported but unused
src/django_ixctl/migrations/0001_initial.py:145:89: E501 line too long (89 > 88 characters)
src/django_ixctl/exporters/ixf.py:7:5: F841 local variable 'ixp_list' is assigned to but never used
src/django_ixctl/models/ixctl.py:25:1: F401 'fullctl.django.models.concrete.APIKey' imported but unused
src/django_ixctl/models/ixctl.py:25:1: F401 'fullctl.django.models.concrete.OrganizationUser' imported but unused
src/django_ixctl/rest/views/ixctl.py:356:68: W605 invalid escape sequence '\d'
src/ixctl/urls.py:4:1: F401 'django_ixctl.urls' imported but unused
src/ixctl/wsgi.py:23:1: E402 module level import not at top of file
src/ixctl/wsgi.py:31:1: E402 module level import not at top of file
src/ixctl/settings/__init__.py:15:1: F401 'django.utils.translation.gettext_lazy as _' imported but unused
src/ixctl/settings/__init__.py:21:8: F821 undefined name 'DEBUG'
src/ixctl/settings/__init__.py:27:25: F821 undefined name 'django'
src/ixctl/settings/__init__.py:47:89: E501 line too long (89 > 88 characters)
src/ixctl/settings/__init__.py:56:89: E501 line too long (136 > 88 characters)
src/ixctl/settings/__init__.py:65:89: E501 line too long (137 > 88 characters)
src/ixctl/settings/__init__.py:83:41: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:106:4: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:112:55: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:116:32: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:147:22: F821 undefined name 'SERVER_EMAIL'
src/ixctl/settings/__init__.py:153:31: F821 undefined name 'PACKAGE_VERSION'
src/ixctl/settings/__init__.py:156:32: F821 undefined name 'PACKAGE_VERSION'
src/ixctl/settings/__init__.py:160:34: F821 undefined name 'SERVER_EMAIL'
src/ixctl/settings/__init__.py:219:41: F821 undefined name 'DATABASE_ENGINE'
src/ixctl/settings/__init__.py:220:17: F821 undefined name 'DATABASE_HOST'
src/ixctl/settings/__init__.py:221:17: F821 undefined name 'DATABASE_PORT'
src/ixctl/settings/__init__.py:222:17: F821 undefined name 'DATABASE_NAME'
src/ixctl/settings/__init__.py:223:17: F821 undefined name 'DATABASE_USER'
src/ixctl/settings/__init__.py:224:21: F821 undefined name 'DATABASE_PASSWORD'
src/ixctl/settings/__init__.py:229:1: E266 too many leading '#' for block comment
src/ixctl/settings/__init__.py:236:89: E501 line too long (90 > 88 characters)
src/ixctl/settings/__init__.py:285:37: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:286:34: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:287:32: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:292:89: E501 line too long (135 > 88 characters)
src/ixctl/settings/__init__.py:295:27: F821 undefined name 'OAUTH_TWENTYC_KEY'
src/ixctl/settings/__init__.py:296:30: F821 undefined name 'OAUTH_TWENTYC_SECRET'
src/ixctl/settings/__init__.py:302:20: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:344:89: E501 line too long (94 > 88 characters)
src/ixctl/settings/__init__.py:361:15: F821 undefined name 'DEBUG'
src/ixctl/settings/__init__.py:363:36: F821 undefined name 'DEBUG'
src/ixctl/settings/__init__.py:365:44: F821 undefined name 'PACKAGE_VERSION'
src/ixctl/settings/__init__.py:365:70: F821 undefined name 'DEBUG'

@vegu maybe we should just chat tomorrow about which of the Flake8 changes to make here.

[vegu]

Yeah lets - generally speaking anything but E501 should be fixed i think. Maybe even have flake8 ignore formatting entirely since black and isort will do their thing with that.

Its commented out in ctl because its failing on a bunch of "functions too complex" alerts - i think, will ask grizz, also curious how to make them play nicely together

[egfrank]

Okay @vegu now that I'm looking into this , there's one category that I don't quite know how to resolve.

Elliots-MacBook-Air:ixctl elliotgoldingfrank$ flake8 src/ixctl/settings/__init__.py
src/ixctl/settings/__init__.py:19:8: F821 undefined name 'DEBUG'
src/ixctl/settings/__init__.py:25:25: F821 undefined name 'django'
src/ixctl/settings/__init__.py:84:41: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:107:4: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:113:55: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:117:32: F821 undefined name 'RELEASE_ENV'
src/ixctl/settings/__init__.py:148:22: F821 undefined name 'SERVER_EMAIL'
src/ixctl/settings/__init__.py:154:31: F821 undefined name 'PACKAGE_VERSION'
src/ixctl/settings/__init__.py:157:32: F821 undefined name 'PACKAGE_VERSION'
src/ixctl/settings/__init__.py:161:34: F821 undefined name 'SERVER_EMAIL'
src/ixctl/settings/__init__.py:220:41: F821 undefined name 'DATABASE_ENGINE'
src/ixctl/settings/__init__.py:221:17: F821 undefined name 'DATABASE_HOST'
src/ixctl/settings/__init__.py:222:17: F821 undefined name 'DATABASE_PORT'
src/ixctl/settings/__init__.py:223:17: F821 undefined name 'DATABASE_NAME'
src/ixctl/settings/__init__.py:224:17: F821 undefined name 'DATABASE_USER'
src/ixctl/settings/__init__.py:225:21: F821 undefined name 'DATABASE_PASSWORD'
src/ixctl/settings/__init__.py:287:37: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:288:34: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:289:32: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:294:27: F821 undefined name 'OAUTH_TWENTYC_KEY'
src/ixctl/settings/__init__.py:295:30: F821 undefined name 'OAUTH_TWENTYC_SECRET'
src/ixctl/settings/__init__.py:301:20: F821 undefined name 'OAUTH_TWENTYC_HOST'
src/ixctl/settings/__init__.py:362:15: F821 undefined name 'DEBUG'
src/ixctl/settings/__init__.py:364:36: F821 undefined name 'DEBUG'
src/ixctl/settings/__init__.py:366:44: F821 undefined name 'PACKAGE_VERSION'
src/ixctl/settings/__init__.py:366:70: F821 undefined name 'DEBUG'

All of these are coming from the fact that we're using global variables in the settings file, I think. Do you have any insight on this?

[egfrank]

Linking to actions for convenience: https://github.com/fullctl/ixctl/actions

Okay @vegu I actually got this repo to the point where the linters are passing: https://github.com/fullctl/ixctl/runs/1780418777?check_suite_focus=true

That being said I added this to tox

per-file-ignores =
    src/ixctl/settings/__init__.py:F821
    src/ixctl/settings/dev.py:F821

so that flake8 would ignore the issue I posted about above

[vegu]

spent a couple minutes looking into the flake8 vs globals thing and seems to be a long known issue that is marked as wont fix on their end.

i dont like that we have to ignore the settings files entirely, but i also dont see a way around it without predefining all globals just to make flake8 happy.

Since the settings functions will go into their own repo soon i am fine to revisit this there if needed and take another look here as well after that. Rest looks good, closing this.