Create lint pipeline and fix errors #586
Conversation
mateusz-twarog-deepsense
changed the title
| - name: Set up Go | ||
| uses: actions/setup-go@v5 | ||
| with: | ||
| go-version: '>=1.22' |
There was a problem hiding this comment.
Any reason we're pinning the python version and not the go version?
There was a problem hiding this comment.
Good point, I must have copy-pasted this config without thinking. In theory it should be fine since go cares about backward compatibility, but pinning makes more sense.
| python -m mypy --ignore-missing-imports --exclude 'racetrack_client/build' racetrack_client; e1=$$?;\ | ||
| python -m mypy --ignore-missing-imports racetrack_commons; e2=$$?;\ | ||
| python -m mypy --ignore-missing-imports --exclude 'lifecycle/lifecycle/django/registry/migrations' lifecycle; e3=$$?;\ | ||
| python -m mypy --ignore-missing-imports image_builder; e4=$$?;\ | ||
| python -m mypy --ignore-missing-imports dashboard; e5=$$?;\ | ||
| python -m flake8 --ignore E501 --per-file-ignores="__init__.py:F401 lifecycle/lifecycle/event_stream/server.py:E402" \ | ||
| lifecycle image_builder dashboard; e6=$$?;\ | ||
| python -m pylint --disable=R,C,W \ | ||
| lifecycle/lifecycle image_builder/image_builder dashboard/dashboard; e7=$$?;\ | ||
| exit "$$(( e1 || e2 || e3 || e4 || e5 || e6 || e7 ))" |
There was a problem hiding this comment.
I'm not enough of a Makefile guru to understand what is going on here. It seems like we previously ignored non-zero exit codes, but now we exit with the cumulative or of every exit code? Why, and what does it even mean to or an exit code - is it just to check if any are truthy (I.E, nonzero)?
Also, does this even work? I was under the impression that Makefiles exited upon a nonzero return code. With the initial dash deleted, that's the case here, no?
There was a problem hiding this comment.
This basically takes all exit codes and "or" on them will return 0 if all of them were 0, some other code if at least one was not 0. It works, but it's ugly - this is purely bash syntax used, since it's makefile maybe I could split it into multiple commands and last one dependent on them? I would need to experiment a bit.
There was a problem hiding this comment.
If it works then I'm not too fussed either way. Makefiles are not exactly known for their graceful syntax
| class Hash(Protocol): | ||
| def __call__(self, string: ReadableBuffer = b"", *, usedforsecurity: bool = True) -> hashlib._Hash: | ||
| ... |
There was a problem hiding this comment.
What does this do except be a placeholder? If it really is only a placeholder, why?
There was a problem hiding this comment.
Ah, this one is weird. There is no type I could import from hash library that would allow me to correctly type digest few lines below, so I had to write correct interface myself.
There was a problem hiding this comment.
Huh. Put a comment in about it, otherwise this just looks wrong.
| def connect(cls, *args, **kwargs) -> "PgConnection": | ||
| try: | ||
| return Connection.connect(*args, **kwargs) | ||
| return super(PgConnection, cls).connect(*args, **kwargs) |
There was a problem hiding this comment.
This again seems like a non-linting change, what are we fixing?
There was a problem hiding this comment.
Typing was incorrect here, because it involved a bit of magic, like connect returning object of its own type. Functionally we are doing exactly the same thing as before, but with a syntax that makes more sense for mypy.
| try: | ||
| user = User.objects.get(username='admin') | ||
| except User.DoesNotExist: | ||
| except User.DoesNotExist: # pylint: disable=E1101 |
There was a problem hiding this comment.
When we ignore the linter, I think a comment explaining why is a good idea.
There was a problem hiding this comment.
Now I regret not making notes when doing this - I will have to go through those ignores one by one.
| method: str = record.args[1] # type: ignore | ||
| uri: str = record.args[2] # type: ignore | ||
| response_code: int = record.args[4] # type: ignore |
There was a problem hiding this comment.
Again, if we're ignoring the linter, I think we should explain why
| return fastapi_app.openapi_schema | ||
|
|
||
| fastapi_app.openapi = custom_openapi | ||
| fastapi_app.openapi = custom_openapi # type: ignore |
There was a problem hiding this comment.
Again, if we're ignoring the linter, I think we should explain why
|
|
||
| metrics_app = make_wsgi_app(REGISTRY) | ||
| api.mount('/metrics', WSGIMiddleware(metrics_app)) | ||
| api.mount('/metrics', WSGIMiddleware(metrics_app)) # type: ignore |
There was a problem hiding this comment.
Again, if we're ignoring the linter, I think we should explain why
| wheel==0.38.4 | ||
| types-PyYAML==6.0.12 | ||
| types-python-dateutil==2.9.0.20250708 | ||
| types-Markdown==3.3.18 |
There was a problem hiding this comment.
No newline at end of file
2 participants
Resolves #581 and #587