Add tox.yml and update tox.ini based on eda-partner-testing #135
Conversation
|
mmm, seems that ruff is raising issues. |
|
@Alex-Izquierdo Yeah, I think we will have to fix all of the issues in this PR since with the new files it's throwing a ton of errors |
|
This change looks like a nice thing to have, maybe due that this will be a fairly huge change, it might be good to split it in multiple PRs to avoid regressions and making it easy to review. Instead of calling all the environments at once in tox, you could make it pass step by step running the environments separately. |
I don't disagree, though there's going to be a ton of changes either way, unfortunately :(. Still, it's probably a good idea to change this one to only be the |
|
Splitting the tox envs in multiple actions it would the best as well to split the updates to the code for an easier review. |
I have something for that from a while a go.. but didnt finish it, let me see if I can find it. Also the tox.ini should not be in the .github/workflows folder. |
Remaining issues in this file: 15:23: FBT001 Boolean positional arg in function definition 15:41: FBT002 Boolean default value in function definition
Remaining issues: 46:23: FBT001 Boolean positional arg in function definition 46:41: FBT002 Boolean default value in function definition 53:41: FBT001 Boolean positional arg in function definition 53:58: ANN001 Missing type annotation for function argument `logger` 69:5: RET505 Unnecessary `elif` after `return` statement
Remaining issues: 45:18: ARG001 Unused function argument: `request` 99:89: E501 Line too long (91 > 88 characters) 133:9: T201 `print` found 144:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 48:5: ANN201 Missing return type annotation for public function `get_events` 48:5: D103 Missing docstring in public function 48:16: ANN001 Missing type annotation for function argument `events` 48:24: ANN001 Missing type annotation for function argument `last_event_ids` 65:11: ANN201 Missing return type annotation for public function `get_cloudtrail_events` 65:11: D103 Missing docstring in public function 65:33: ANN001 Missing type annotation for function argument `client` 65:41: ANN001 Missing type annotation for function argument `params` 87:27: DTZ003 The use of `datetime.datetime.utcnow()` is not allowed, use `datetime.datetime.now(tz=)` instead 133:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 52:17: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling 52:17: TRY200 Use `raise from` to specify exception cause 111:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 66:30: ANN101 Missing type annotation for `self` in method
Remaining issues: 25:16: ANN001 Missing type annotation for function argument `queue` 27:10: PTH123 `open()` should be replaced by `Path.open()` 35:89: E501 Line too long (91 > 88 characters) 36:17: TRY004 Prefer `TypeError` exception for invalid type 36:23: TRY002 Create your own exception 41:23: TRY002 Create your own exception 48:5: C901 `main` is too complex (11 > 10) 48:10: ANN001 Missing type annotation for function argument `queue` 50:14: PTH100 `os.path.abspath()` should be replaced by `Path.resolve()` 59:22: ANN101 Missing type annotation for `self` in method 59:30: ANN003 Missing type annotation for `**kwargs` 62:24: ANN101 Missing type annotation for `self` in method 66:24: ANN101 Missing type annotation for `self` in method 69:25: ANN101 Missing type annotation for `self` in method 73:22: ANN101 Missing type annotation for `self` in method 80:21: PTH120 `os.path.dirname()` should be replaced by `Path.parent` 97:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 28:89: E501 Line too long (92 > 88 characters) 33:22: ANN101 Missing type annotation for `self` in method 33:30: ANN003 Missing type annotation for `**kwargs` 36:24: ANN101 Missing type annotation for `self` in method 47:24: ANN101 Missing type annotation for `self` in method 58:25: ANN101 Missing type annotation for `self` in method 69:22: ANN101 Missing type annotation for `self` in method 106:30: ANN101 Missing type annotation for `self` in method 110:44: S108 Probable insecure usage of temporary file or directory: "/tmp"
Remaining issues: 51:11: C901 `main` is too complex (15 > 10) 51:11: PLR0912 Too many branches (14 > 12) 94:49: DTZ005 The use of `datetime.datetime.now()` without `tz` argument is not allowed 98:45: DTZ005 The use of `datetime.datetime.now()` without `tz` argument is not allowed 103:21: T201 `print` found 118:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 63:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 54:15: TRY002 Create your own exception 87:17: TRY400 Use `logging.exception` instead of `logging.error` 103:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 36:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 36:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 74:23: ANN101 Missing type annotation for `self` in method
Remaining issues: 45:11: ANN201 Missing return type annotation for public function `bearer_auth` 50:13: TRY301 Abstract `raise` to an inner function 51:9: RET506 Unnecessary `elif` after `raise` statement 52:13: TRY301 Abstract `raise` to an inner function 54:9: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling 54:9: TRY200 Use `raise from` to specify exception cause 56:9: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling 56:9: TRY200 Use `raise from` to specify exception cause 84:13: TRY400 Use `logging.exception` instead of `logging.error` 91:29: S104 Possible binding to all interfaces 111:23: ANN101 Missing type annotation for `self` in method
I'm simply following the instructions from https://github.com/ansible/eda-partner-testing/blob/main/README.md#toxini which says it should be in that folder. |
|
Alright, I've updated this PR to just run the ruff check, which should make this a bit more manageable. Future PRs will enable the others that we need ( I'm going to need some help to resolve all of these as for some of them I just don't have the knowledge. If you're tagged here, please take a look at the changes made to the file that I did as some of them were done automatically via a I've also created a commit for each file as most of them had remaining issues, and I listed the remaining issues I need assistance with in the commit message, hopefully that helps make it a bit easier to isolate, though I acknowledge it's probably a lot of noise and can be condensed later. Here's a list of all the files that need to be adjusted, along with the majority committer(s). |
|
Something like this @eclarizio #138 which will enable all the people who wants to make the lint checks consistent without breaking any job. The initial goal of that PR was to move tox.ini to the root of the collection, running tox is not specific to github actions so it shouldnt be there. |
I think that is a reference guide for the tests that should be executed, for example the tox.ini file is not exclusive of github actions so it should not be there. If a partner uses i.e. GitLab, Zuul, Jenkins, or Travis this might not apply. |
|
I like a lot the new styling proposed here, but I was testing this PR locally and mixing darglint with ruff will be a endless party as some rules conflicts each other xD |
|
@eclarizio I will address most of the errors |
| class Handler(RegexMatchingEventHandler): | ||
| def __init__(self, **kwargs) -> None: | ||
| def __init__(self: "Handler", **kwargs) -> None: # noqa: ANN003 |
There was a problem hiding this comment.
Can we not use the same type as here e7f4d98#diff-a614b4efc2dd1f32c82962f861ab13d16ac3a843d8b2c38a6136b21dcc3a8421R37?
There was a problem hiding this comment.
I believe this file is not compatible with ansible-rulebook engine. It should not be used. I put some simple fix to suppress the warning. This file should be removed or rewritten.
bzwei
force-pushed
the
update_tox_workflows
branch
from
e7f4d98 to
3d67cd6
Compare
bzwei
force-pushed
the
update_tox_workflows
branch
from
3d67cd6 to
55d18b3
Compare
|
@eclarizio The test failed with python3.8 because syntax like dict[str, Any] is not supported. Should we fix them or just drop version 3.8? |
|
@bzwei @Alex-Izquierdo I think I would propose we drop 3.8. According to this link Python 3.9 in RHEL is going EOL in May 2024, and Python 3.8 is already not supported. That's for RHEL 8, and for RHEL 9 it looks like we want to be using 3.11. |
|
At some point I remember to remove or propose the removal of python3.8 but @mkanoor had some reason to keep it. I don't remember well. For me, totally agree to remove it. |
We needed to update the tox files as a requirement for certification