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

add Ruff for Python file format and lint #1584

Merged
merged 13 commits into from
Sep 25, 2024
2 changes: 1 addition & 1 deletion .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
- name: Install dependencies
run: |
apk update && apk add cppcheck python3-dev
python3 -m pip install cmake-format clang-format==18.1.6
python3 -m pip install cmake-format clang-format==18.1.6 ruff==0.6.5
Copy link
Owner

Choose a reason for hiding this comment

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

Now that we use ruff-pre-commit I don't think this is needed


# TODO: investigate how to run pre-commit with `venv`
- uses: pre-commit/action@2c7b3805fd2a0fd8c1884dcaebf91fc102a13ecd # v3.0.1
Expand Down
18 changes: 14 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ repos:
name: Check clang-format version
entry: python3 ./ci/check-clang-format-version.py
language: system
- id: check-ruff-format-version
name: Check ruff-format version
entry: python3 ./ci/check-ruff-version.py
language: system
Copy link
Owner

Choose a reason for hiding this comment

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

Now that we use ruff-pre-commit this is not needed

- id: ruff
name: ruff
entry: ruff check
types_or: [python]
language: system
- id: ruff-format
name: ruff-format
entry: ruff format --check
types_or: [python]
language: system
Copy link
Owner

Choose a reason for hiding this comment

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

Shouldn't we use ruff-pre-commit?

It can make it easier to upgrade ruff if we need to

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Originally I think it's more flexible to do it on our side, but I think it also works to use other's scripts.

- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.6.0
hooks:
Expand All @@ -21,10 +35,6 @@ repos:
- id: mixed-line-ending
args: ['--fix=lf']
- id: trailing-whitespace
- repo: https://github.com/psf/black
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no need black now.

rev: 24.8.0
hooks:
- id: black
- repo: https://github.com/pocc/pre-commit-hooks
rev: v1.3.5
hooks:
Expand Down
2 changes: 1 addition & 1 deletion 3rdParty/OUIDataset/create_oui_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def convert_line(line: str) -> list[str]:


def parse_mac_and_vendor(line_parts: list[str]) -> Optional[LineElements]:
if line_parts == None or len(line_parts) < 3:
if line_parts is None or len(line_parts) < 3:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lint errors from Ruff.

return None

if len(line_parts[0]) == 6:
Expand Down
1 change: 0 additions & 1 deletion Tests/ExamplesTest/tests/test_httpanalyzer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from os import path
from itertools import filterfalse
import pytest
from .test_utils import ExampleTest, compare_stdout_with_file

Expand Down
6 changes: 2 additions & 4 deletions Tests/ExamplesTest/tests/test_pcapsearch.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
from os import path
import pytest
import re
import ntpath
from .test_utils import ExampleTest

Expand Down Expand Up @@ -42,7 +40,7 @@ def test_exact_file_format(self):
num_of_packets = int(words[0])
file_name = ntpath.basename(words[-1].replace("'", ""))
actual.add((num_of_packets, file_name))
except:
except Exception:
pass

assert expected.issubset(actual)
Expand All @@ -52,7 +50,7 @@ def test_different_file_extensions(self):
completed_process = self.run_example(args=args)
assert ".dmp'" in completed_process.stdout
assert ".pcapng'" in completed_process.stdout
assert not ".pcap'" in completed_process.stdout
assert ".pcap'" not in completed_process.stdout

def test_no_args(self):
args = {}
Expand Down
2 changes: 1 addition & 1 deletion Tests/ExamplesTest/tests/test_pcapsplitter.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def test_split_by_connection(self, tmpdir):
else:
conn = frozenset([])

assert not conn in connection_map
assert conn not in connection_map
connection_map[conn] = True

if len(conn) == 0:
Expand Down
1 change: 0 additions & 1 deletion Tests/ExamplesTest/tests/test_sslanalyzer.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from os import path
from itertools import filterfalse
import pytest
from .test_utils import ExampleTest, compare_stdout_with_file

Expand Down
1 change: 0 additions & 1 deletion Tests/ExamplesTest/tests/test_tlsfingerprinting.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import os
import filecmp
import pytest
from .test_utils import (
ExampleTest,
Expand Down
18 changes: 18 additions & 0 deletions ci/check-ruff-version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import subprocess

EXPECTED_RUFF_VERSION = "0.6.5"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

check the version.


def main():
result = subprocess.run(("ruff", "--version"), capture_output=True)
result.check_returncode()

version_str = result.stdout.decode("utf-8").split(" ")[1].strip()
if version_str != EXPECTED_RUFF_VERSION:
raise ValueError(
f"Error: Found ruff version {version_str}, but {EXPECTED_RUFF_VERSION} is required."
)


if __name__ == "__main__":
main()
2 changes: 1 addition & 1 deletion ci/run_tests/run_tests_windows.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ def find_interface():
if completed_process.returncode != 0:
continue
return interface, ip_address
except:
except Exception:
pass
return None, None

Expand Down
24 changes: 11 additions & 13 deletions cmake/setup_dpdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,7 @@ def usage():
[Restore]

sudo python setup_dpdk.py restore\n\n
""".format(
settings_file=_SETTINGS_FILE
)
""".format(settings_file=_SETTINGS_FILE)


# This is roughly compatible with check_output function in subprocess module
Expand Down Expand Up @@ -439,7 +437,7 @@ def handle_error(error_msg):
filename = "/sys/bus/pci/drivers/%s/unbind" % dev["Driver_str"]
try:
file_d = open(filename, "a")
except: # pylint:disable=bare-except
except Exception:
handle_error(
"Error: unbind failed for %s - Cannot open %s" % (dev_id, filename)
)
Expand Down Expand Up @@ -491,7 +489,7 @@ def handle_error(error_msg):
if os.path.exists(filename):
try:
file_d = open(filename, "w")
except: # pylint:disable=bare-except
except Exception:
handle_error(
"Error: bind failed for %s - Cannot open %s" % (dev_id, filename)
)
Expand All @@ -500,7 +498,7 @@ def handle_error(error_msg):
file_d.write("%s" % driver)
logger.debug("bind_one: write '%s' to '%s'", driver, filename)
file_d.close()
except: # pylint:disable=bare-except
except Exception:
handle_error(
"Error: bind failed for %s - Cannot write driver %s to "
"PCI ID " % (dev_id, driver)
Expand All @@ -511,7 +509,7 @@ def handle_error(error_msg):
filename = "/sys/bus/pci/drivers/%s/new_id" % driver
try:
file_d = open(filename, "w")
except: # pylint:disable=bare-except
except Exception:
handle_error(
"Error: bind failed for %s - Cannot open %s" % (dev_id, filename)
)
Expand All @@ -520,7 +518,7 @@ def handle_error(error_msg):
# Convert Device and Vendor Id to int to write to new_id
file_d.write("%04x %04x" % (int(dev["Vendor"], 16), int(dev["Device"], 16)))
file_d.close()
except: # pylint:disable=bare-except
except Exception:
handle_error(
"Error: bind failed for %s - Cannot write new PCI ID to "
"driver %s" % (dev_id, driver)
Expand All @@ -531,7 +529,7 @@ def handle_error(error_msg):
filename = "/sys/bus/pci/drivers/%s/bind" % driver
try:
file_d = open(filename, "a")
except: # pylint:disable=bare-except
except Exception:
logger.error("Error: bind failed for %s - Cannot open %s", dev_id, filename)
if saved_driver is not None: # restore any previous driver
bind_one(dev_id, saved_driver, quiet, force)
Expand All @@ -540,7 +538,7 @@ def handle_error(error_msg):
file_d.write(dev_id)
logger.debug("bind_one: write '%s' to '%s'", dev_id, filename)
file_d.close()
except: # pylint:disable=bare-except
except Exception:
# for some reason, closing dev_id after adding a new PCI ID to new_id
# results in IOError. however, if the device was successfully bound,
# we don't care for any errors and can safely ignore IOError
Expand All @@ -561,7 +559,7 @@ def handle_error(error_msg):
if os.path.exists(filename):
try:
file_d = open(filename, "w")
except: # pylint:disable=bare-except
except Exception:
handle_error(
"Error: unbind failed for %s - Cannot open %s" % (dev_id, filename)
)
Expand All @@ -570,7 +568,7 @@ def handle_error(error_msg):
file_d.write("\00")
logger.debug("bind_one: write '\00' to '%s'", filename)
file_d.close()
except: # pylint:disable=bare-except
except Exception:
handle_error(
"Error: unbind failed for %s - Cannot open %s" % (dev_id, filename)
)
Expand Down Expand Up @@ -1216,7 +1214,7 @@ def main():
try:
args.func(args, settings)
settings.save(settings_file_full_path)
except: # pylint:disable=bare-except
except Exception:
pass


Expand Down
Loading