chore: project maintenance and quality improvements #5
Conversation
|
Warning Rate limit exceeded@ahsentekd has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis update introduces extensive modernization and restructuring across the ZMap SDK project. The core codebase is refactored to use modern Python 3.10+ type hint syntax, centralizes Pydantic schema definitions, and migrates the CLI from Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI (click)
participant APIServer
participant ZMap
participant Runner
participant Schemas
User->>CLI (click): Invoke CLI command (e.g., api)
CLI (click)->>APIServer: Start API server with options
APIServer->>Schemas: Validate request with Pydantic models
APIServer->>ZMap: Initiate scan or blocklist/allowlist action
ZMap->>Runner: Run scan command with config
Runner-->>ZMap: Return scan results
ZMap-->>APIServer: Return results or file path
APIServer-->>User: Respond with scan results or file
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (32)
CODE_OF_CONDUCT.md (2)
61-61: Shorten “with regard to”
Consider replacing “with regard to” with “regarding” or “about” to improve conciseness:- confidentiality with regard to the reporter… + confidentiality regarding the reporter…🧰 Tools
🪛 LanguageTool
[style] ~61-~61: ‘with regard to’ might be wordy. Consider a shorter alternative.
Context: ...s obligated to maintain confidentiality with regard to the reporter of an incident. Further de...(EN_WORDINESS_PREMIUM_WITH_REGARD_TO)
65-65: Add comma for readability
Insert a comma after “in good faith” to clearly separate the subordinate clause:- enforce the Code of Conduct in good faith may face… + enforce the Code of Conduct in good faith, may face…🧰 Tools
🪛 LanguageTool
[uncategorized] ~65-~65: Possible missing comma found.
Context: ...d faith may face temporary or permanent repercussions as determined by other members of the p...(AI_HYDRA_LEO_MISSING_COMMA)
build-package.sh (1)
17-17: Informative completion & error handling
Echoing completion is helpful. For robustness, consider adding at the top:set -euo pipefailThis ensures the script exits on any failure, preventing a misleading “complete” message if a step fails.
docs/index.rst (1)
14-17: Populatetoctreewith documentation pages.
The currenttoctreedirective has no entries; consider adding references to your documentation files (e.g.,usage,api/modules) to enable proper navigation in the generated docs..github/workflows/ci.yml (2)
13-15: Expand Python version coverage.
To ensure compatibility with future releases, consider testing additional supported Python versions (e.g.,3.11) by extending thematrix.python-version.
24-30: Cache Poetry dependencies to speed up CI.
Consider adding anactions/cache@v3step for the Poetry virtual environment (.venv) or the Poetry cache directory to reduce install times on subsequent runs.Makefile (2)
7-10: Include tests in formatting
Theformattarget only runsisortandblackonzmapsdk. For consistent style across the repo, consider addingtests/(and any other Python directories) to these commands.format: - poetry run isort zmapsdk - poetry run black zmapsdk + poetry run isort zmapsdk tests + poetry run black zmapsdk tests
11-14: Scope lint checks to source and tests
Running checks on.may include unintended files (e.g., CI configs). Restrict lint targets to your code directories:lint: - poetry run isort --check-only --diff . - poetry run black --check . + poetry run isort --check-only --diff zmapsdk tests + poetry run black --check zmapsdk tests poetry run pyupgrade --py311-plus $$(find . -name '*.py' -not -path './.venv/*')docs/Makefile (1)
6-9: Define or remove the unusedOvariable
The Makefile references$(O)for extra Sphinx options, butOisn’t defined. You can add a definition (O ?=) or remove$(O)to avoid confusion.SPHINXOPTS ?= SPHINXBUILD ?= sphinx-build +O ?=setup.cfg (1)
2-2: Refinepython_filespattern
The patterntest*.pymay match unintended files. Consider using the more conventionaltest_*.pyfor pytest discovery.-python_files = test*.py +python_files = test_*.pydocs/make.bat (1)
7-22: Useecho(for reliable blank lines
In batch scripts,echo.can be ambiguous. Switching toecho(ensures a blank line is printed even if a file namedechoexists.-if errorlevel 9009 ( - echo. - echo.The 'sphinx-build' command was not found. Make sure you have Sphinx +if errorlevel 9009 ( + echo( + echo(The 'sphinx-build' command was not found. Make sure you have Sphinx echo.installed, then set the SPHINXBUILD environment variable to point echo.to the full path of the 'sphinx-build' executable. Alternatively you echo.may add the Sphinx directory to PATH.docs/conf.py (1)
1-53: Well-structured Sphinx documentation configurationThe Sphinx configuration is set up correctly with appropriate extensions and project metadata extraction from pyproject.toml.
I particularly like:
- The helper function to extract project details from pyproject.toml
- The comprehensive set of extensions (autodoc, viewcode, doctest, autoapi)
- The appropriate configuration of autoapi to scan the zmapsdk directory
One thing to consider:
Consider adding a requirements file for documentation dependencies to make it easier for contributors to build the docs. This could be added as a dependency group in your pyproject.toml:
+ [tool.poetry.group.docs.dependencies] + sphinx = "^7.2.6" + sphinx-rtd-theme = "^1.3.0" + sphinx-autoapi = "^3.0.0" + tomlkit = "^0.12.3"zmapsdk/config.py (1)
115-123: Simplify nested conditionals and improve exception handlingThe static analyzer suggests two improvements here:
- if self.max_targets is not None and isinstance(self.max_targets, str): - if not self.max_targets.endswith("%"): - try: - int(self.max_targets) - except ValueError: - raise ZMapConfigError( - f"Invalid max_targets: {self.max_targets}. Must be an integer or percentage.", - ) + if self.max_targets is not None and isinstance(self.max_targets, str) and not self.max_targets.endswith("%"): + try: + int(self.max_targets) + except ValueError as e: + raise ZMapConfigError( + f"Invalid max_targets: {self.max_targets}. Must be an integer or percentage.", + ) from eThis change:
- Combines the nested conditions into a single line
- Preserves the original exception traceback with
from e🧰 Tools
🪛 Ruff (0.8.2)
115-116: Use a single
ifstatement instead of nestedifstatements(SIM102)
120-122: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
zmapsdk/schemas.py (3)
19-19: Consider documenting extension pointsThe comment suggests that more parameters could be added, but doesn't provide guidance on how to extend the schema appropriately.
Consider replacing this with a more descriptive comment that guides future developers:
- # Add other relevant parameters as needed + # Additional parameters can be added here. + # For ZMap parameters reference, see: https://github.com/zmap/zmap/wiki/Command-Line-Arguments
22-28: Consider adding field descriptions for ScanResultUnlike
ScanRequest, theScanResultmodel lacks field descriptions, which would be valuable for API documentation.class ScanResult(BaseModel): - scan_id: str - status: str - ips_found: list[str] | None = None - output_file: list[str] | None = None - error: list[str] | None = None + scan_id: str = Field(..., description="Unique identifier for the scan") + status: str = Field(..., description="Current status of the scan (e.g., completed, failed)") + ips_found: list[str] | None = Field(None, description="List of IP addresses discovered in the scan") + output_file: list[str] | None = Field(None, description="Path(s) to output file(s)") + error: list[str] | None = Field(None, description="Error messages if scan failed")
30-42: Add field descriptions to remaining modelsSimilar to
ScanResult, the other models would benefit from field descriptions.class BlocklistRequest(BaseModel): - subnets: list[str] - output_file: str | None = None + subnets: list[str] = Field(..., description="List of subnets to blocklist") + output_file: str | None = Field(None, description="Path to save the blocklist") class StandardBlocklistRequest(BaseModel): - output_file: str | None = None + output_file: str | None = Field(None, description="Path to save the standard blocklist") class FileResponse(BaseModel): - file_path: str - message: str + file_path: str = Field(..., description="Path to the generated file") + message: str = Field(..., description="Status or result message")pyproject.toml (1)
34-36: Consider adding comments for new dependenciesFor better maintainability, consider adding comments to explain why certain dependencies were added (especially newer ones like tomli, tomlkit, and httpx).
python = ">=3.10" fastapi = ">=0.100.0" uvicorn = ">=0.22.0" netifaces = ">=0.11.0" click = ">=8.1.8,<9.0.0" # Used for CLI interface tomli = ">=2.0.1,<3.0.0" # TOML parsing support for Python < 3.11 tomlkit = ">=0.13.2,<0.14.0" # Read/write TOML with formatting preservation httpx = ">=0.28.1,<0.29.0" # Modern HTTP client for API testingzmapsdk/parser.py (7)
76-77: Follow Python best practices for chained exceptionsThe exception handling here should preserve the original exception context using
raise ... from esyntax.- except (csv.Error, OSError) as e: - raise ZMapParserError(f"Failed to parse CSV results: {e!s}") + except (csv.Error, OSError) as e: + raise ZMapParserError(f"Failed to parse CSV results: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
77-77: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
108-109: Follow Python best practices for chained exceptionsSimilar to the previous case, preserve the exception context.
- except (json.JSONDecodeError, OSError) as e: - raise ZMapParserError(f"Failed to parse JSON results: {e!s}") + except (json.JSONDecodeError, OSError) as e: + raise ZMapParserError(f"Failed to parse JSON results: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
109-109: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
132-133: Follow Python best practices for chained exceptionsConsistently apply exception chaining throughout the codebase.
- except (json.JSONDecodeError, OSError) as e: - raise ZMapParserError(f"Failed to parse metadata: {e!s}") + except (json.JSONDecodeError, OSError) as e: + raise ZMapParserError(f"Failed to parse metadata: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
133-133: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
160-163: Consider using contextlib.suppress for cleaner error suppressionWhen intentionally ignoring exceptions,
contextlib.suppressprovides a cleaner approach.- try: - row[key] = float(value) if "." in value else int(value) - except ValueError: - pass # Keep as string if conversion fails + from contextlib import suppress + with suppress(ValueError): + row[key] = float(value) if "." in value else int(value) + # Keep as string if conversion fails🧰 Tools
🪛 Ruff (0.8.2)
160-163: Use
contextlib.suppress(ValueError)instead oftry-except-pass(SIM105)
168-169: Follow Python best practices for chained exceptionsApply exception chaining to maintain the error trace.
- except (csv.Error, OSError) as e: - raise ZMapParserError(f"Failed to parse status updates: {e!s}") + except (csv.Error, OSError) as e: + raise ZMapParserError(f"Failed to parse status updates: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
169-169: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
246-247: Follow Python best practices for chained exceptionsApply exception chaining to maintain the error trace.
- except (csv.Error, OSError) as e: - raise ZMapParserError(f"Failed to parse CSV results: {e!s}") + except (csv.Error, OSError) as e: + raise ZMapParserError(f"Failed to parse CSV results: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
247-247: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
281-282: Follow Python best practices for chained exceptionsComplete the pattern of using exception chaining throughout the codebase.
- except OSError as e: - raise ZMapParserError(f"Failed to read results file: {e!s}") + except OSError as e: + raise ZMapParserError(f"Failed to read results file: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
282-282: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
zmapsdk/api.py (5)
8-8: Remove unused importThe
FileResponsefromfastapi.responsesis imported but never used. The code uses the customFileResponsefromzmapsdk.schemasinstead.-from fastapi.responses import FileResponse as StarletteFileResponse🧰 Tools
🪛 Ruff (0.8.2)
8-8:
fastapi.responses.FileResponseimported but unusedRemove unused import:
fastapi.responses.FileResponse(F401)
96-96: Follow Python best practices for chained exceptionsPreserve exception context using
raise ... from e.- raise HTTPException(status_code=500, detail=str(e)) + raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
96-96: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
120-120: Follow Python best practices for chained exceptionsPreserve exception context consistently across all error handlers.
- raise HTTPException(status_code=500, detail=str(e)) + raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
120-120: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
143-143: Follow Python best practices for chained exceptionsComplete the pattern of using exception chaining throughout the codebase.
- raise HTTPException(status_code=500, detail=str(e)) + raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
143-143: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
176-176: Follow Python best practices for chained exceptionsApply exception chaining to maintain the error trace in the scan-sync endpoint too.
- raise HTTPException(status_code=500, detail=str(e)) + raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
176-176: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
zmapsdk/runner.py (3)
40-40: Follow Python best practices for chained exceptionsPreserve the original exception context using
raise ... from esyntax.- raise ZMapCommandError(command=self.zmap_path, returncode=-1, stderr=str(e)) + raise ZMapCommandError(command=self.zmap_path, returncode=-1, stderr=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
40-40: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
172-172: Follow Python best practices for chained exceptionsPreserve exception context for better debugging.
- raise ZMapCommandError(command=" ".join(cmd), returncode=-1, stderr=str(e)) + raise ZMapCommandError(command=" ".join(cmd), returncode=-1, stderr=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
172-172: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
249-252: Consider using contextlib.suppress for cleaner error handlingWhen intentionally ignoring OSError exceptions during cleanup,
contextlib.suppressprovides a cleaner approach.- try: - os.unlink(temp_file) - except OSError: - pass + from contextlib import suppress + with suppress(OSError): + os.unlink(temp_file)🧰 Tools
🪛 Ruff (0.8.2)
249-252: Use
contextlib.suppress(OSError)instead oftry-except-passReplace with
contextlib.suppress(OSError)(SIM105)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks).pre-commit-config.yaml(1 hunks)CODE_OF_CONDUCT.md(1 hunks)Makefile(1 hunks)__init__.py(1 hunks)build-package.sh(1 hunks)docs/Makefile(1 hunks)docs/conf.py(1 hunks)docs/index.rst(1 hunks)docs/make.bat(1 hunks)examples/advanced-scan.py(2 hunks)pyproject.toml(1 hunks)setup.cfg(1 hunks)setup.py(3 hunks)zmapsdk/__init__.py(1 hunks)zmapsdk/__main__.py(1 hunks)zmapsdk/api.py(5 hunks)zmapsdk/cli.py(1 hunks)zmapsdk/config.py(1 hunks)zmapsdk/core.py(1 hunks)zmapsdk/exceptions.py(2 hunks)zmapsdk/input.py(1 hunks)zmapsdk/output.py(1 hunks)zmapsdk/parser.py(6 hunks)zmapsdk/runner.py(13 hunks)zmapsdk/schemas.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
zmapsdk/__main__.py (1)
zmapsdk/cli.py (2)
cli(45-47)main(80-82)
zmapsdk/config.py (3)
zmapsdk/exceptions.py (1)
ZMapConfigError(23-26)zmapsdk/input.py (1)
to_dict(221-243)zmapsdk/output.py (1)
to_dict(178-218)
zmapsdk/input.py (3)
zmapsdk/exceptions.py (1)
ZMapInputError(29-32)zmapsdk/core.py (4)
create_blocklist_file(212-223)create_allowlist_file(225-236)create_target_file(238-249)generate_standard_blocklist(251-261)zmapsdk/api.py (1)
generate_standard_blocklist(100-120)
zmapsdk/output.py (3)
zmapsdk/exceptions.py (1)
ZMapOutputError(35-38)zmapsdk/config.py (1)
to_dict(139-145)zmapsdk/input.py (1)
to_dict(221-243)
zmapsdk/parser.py (2)
zmapsdk/exceptions.py (1)
ZMapParserError(41-44)zmapsdk/core.py (4)
parse_metadata(280-290)extract_ips(292-307)stream_results(309-320)count_results(322-332)
zmapsdk/core.py (5)
zmapsdk/config.py (1)
ZMapScanConfig(13-170)zmapsdk/input.py (8)
ZMapInput(12-243)add_subnets(42-53)set_blocklist_file(55-69)set_allowlist_file(71-85)create_blocklist_file(103-132)create_allowlist_file(134-163)create_target_file(165-194)generate_standard_blocklist(196-219)zmapsdk/output.py (2)
ZMapOutput(11-247)set_output_file(31-46)zmapsdk/parser.py (6)
ZMapParser(14-282)parse_csv_results(20-77)parse_metadata(112-133)extract_ips(172-192)stream_results(195-247)count_results(250-282)zmapsdk/runner.py (8)
ZMapRunner(16-434)scan(174-252)run_command(78-172)get_probe_modules(254-280)get_output_modules(282-308)get_output_fields(310-343)get_interfaces(345-407)get_version(409-434)
zmapsdk/runner.py (6)
zmapsdk/config.py (2)
ZMapScanConfig(13-170)to_dict(139-145)zmapsdk/exceptions.py (1)
ZMapCommandError(12-20)zmapsdk/api.py (5)
run(196-200)get_probe_modules(53-55)get_output_modules(59-61)get_output_fields(65-67)get_interfaces(71-73)zmapsdk/core.py (6)
run(134-144)scan(63-108)get_probe_modules(146-153)get_output_modules(155-162)get_output_fields(164-174)get_interfaces(176-183)zmapsdk/input.py (2)
ZMapInput(12-243)to_dict(221-243)zmapsdk/output.py (3)
ZMapOutput(11-247)to_dict(178-218)set_output_file(31-46)
🪛 LanguageTool
CODE_OF_CONDUCT.md
[style] ~27-~27: Try using a synonym here to strengthen your wording.
Context: ...vances * Trolling, insulting/derogatory comments, and personal or political attacks * Pu...
(COMMENT_REMARK)
[style] ~61-~61: ‘with regard to’ might be wordy. Consider a shorter alternative.
Context: ...s obligated to maintain confidentiality with regard to the reporter of an incident. Further de...
(EN_WORDINESS_PREMIUM_WITH_REGARD_TO)
[uncategorized] ~65-~65: Possible missing comma found.
Context: ...d faith may face temporary or permanent repercussions as determined by other members of the p...
(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Ruff (0.8.2)
zmapsdk/config.py
115-116: Use a single if statement instead of nested if statements
(SIM102)
120-122: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
zmapsdk/input.py
40-40: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
122-122: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
129-129: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
153-153: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
160-160: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
184-184: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
191-191: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
zmapsdk/api.py
8-8: fastapi.responses.FileResponse imported but unused
Remove unused import: fastapi.responses.FileResponse
(F401)
96-96: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
120-120: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
zmapsdk/parser.py
77-77: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
109-109: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
133-133: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
160-163: Use contextlib.suppress(ValueError) instead of try-except-pass
(SIM105)
169-169: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
192-192: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
247-247: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
282-282: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
zmapsdk/runner.py
40-40: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
172-172: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
249-252: Use contextlib.suppress(OSError) instead of try-except-pass
Replace with contextlib.suppress(OSError)
(SIM105)
🔇 Additional comments (56)
.gitignore (1)
45-47: Ignore Sphinx build artifacts
Addingdocs/_build/ensures that generated documentation output isn’t accidentally committed.zmapsdk/__main__.py (2)
6-6: Group imports with a blank line
Inserting a blank line afterimport sysimproves readability and aligns with PEP8 import grouping (stdlib vs. local imports).
10-10: Remove trailing whitespace & ensure EOF newline
Stripping trailing spaces fromsys.exit(main())and adding a newline at EOF enhances consistency and avoids editor warnings.setup.py (4)
1-1: Update import ordering
Swapping tofrom setuptools import find_packages, setupaligns with alphabetical ordering. Confirm it matches the project’s import style.
3-3: Simplify file open call
Removing the explicit"r"mode relies on the default and makes the code more concise without changing behavior.
16-17: Align comment spacing
Adjusting whitespace around comments forfastapianduvicornentries keeps the list visually consistent.
46-46: Ensure newline & remove trailing whitespace
Adding a final newline and stripping trailing spaces conforms to POSIX and prevents potential diff noise.zmapsdk/exceptions.py (3)
5-14: Improve readability with consistent class separation.
Inserted blank lines before and after theZMapErrorandZMapCommandErrordefinitions ensure a uniform visual separation between exception classes.
23-32: Maintain consistent spacing between exception classes.
Added blank lines aroundZMapConfigErrorandZMapInputErrorto align with the spacing style of adjacent classes.
35-44: Ensure uniform formatting across all exception definitions.
Blank lines added beforeZMapOutputErrorand thepassinZMapParserErroruphold the repository's formatting standards.examples/advanced-scan.py (1)
5-10: Add trailing commas for consistency.
The trailing commas afterrate=1000andretries=2improve git diff clarity and align with the project's code style guidelines.docs/index.rst (1)
1-5: Initial Sphinx master document added.
Theindex.rstfile serves as the entry point for the documentation and follows standard Sphinx layout.zmapsdk/__init__.py (2)
5-16: Reorder and group imports for readability.
The updated import order groups the API server and CLI entry point at the top, followed by core components and exception classes, improving logical structure without altering functionality.
22-38: Align__all__ordering with import order.
The__all__list has been reformatted and reordered to exactly match the import sequence, ensuring consistency in the public API definition..github/workflows/ci.yml (1)
1-12: Add basic CI workflow with Poetry integration.
This workflow sets up Python 3.10, installs Poetry, and runs linting and tests, aligning with the project's migration to Poetry for dependency management.Makefile (1)
16-21: Clean target properly removes build artifacts
Thecleantarget covers build/, dist/, egg-info,__pycache__, and.pycfiles as expected.docs/Makefile (1)
1-21: Documentation Makefile is well-structured
This file provides a minimal, cross-platform Sphinx build setup. Thehelpand wildcard targets follow best practices for documenting available commands.setup.cfg (1)
1-28: Centralized tool configurations improve consistency
The newsetup.cfgcleanly consolidates pytest, coverage, Black, and isort settings, aligning with the project’s Python 3.11 targets..pre-commit-config.yaml (1)
1-23: Pre-commit hooks align with CI and formatting standards
Configs pin specific versions for pre-commit-hooks, Black, isort, and pyupgrade (Python 3.11+), matching the GitHub Actions workflow and local tooling.docs/make.bat (1)
1-36: Windows doc build script is comprehensive
The batch file correctly handles default vars, existence checks, target dispatch, and cleanup—ensuring cross-platform documentation builds.__init__.py (1)
1-21: Improved version management with dynamic loading from pyproject.toml!This is a good update that aligns with modern Python packaging practices. The code now dynamically reads the package version from pyproject.toml, which is the standard approach when using Poetry.
A few observations:
- Good error handling with proper exception catching
- Sensible fallback to "0.0.0" when the version can't be determined
- Clear comments explaining the purpose of the code
zmapsdk/input.py (1)
19-24: Type annotations updated to modern Python 3.10+ syntaxThe transition to using union types with the pipe operator (
|) instead ofOptionalis a good modernization.zmapsdk/config.py (3)
48-80: Clean type hint modernization with union typesThe refactoring of type hints to use the union operator (
|) instead ofOptionalmakes the code more readable and aligns with modern Python 3.10+ practices.
147-150: Well-structured JSON serialization methodsThe serialization and deserialization methods are well-implemented with good use of class methods for construction from external sources.
161-170: File operation methods with proper context managersThe file I/O methods correctly use context managers for file operations, ensuring that resources are properly cleaned up.
zmapsdk/output.py (7)
6-6: Type imports simplifiedGood job simplifying the imports by only including the
Anytype from typing, as the Python 3.10+ union syntax (|) has replaced the need forOptionalandUnionimports.
18-28: Modern type annotations correctly implementedGreat job updating all attribute type annotations to use the Python 3.10+ union syntax (
str | Noneinstead ofOptional[str]). This modernization makes the code more readable and future-proof.
48-48: Type annotation modernized for method parameterThe parameter type annotation has been correctly updated to use Python 3.10+ union syntax (
list[str] | strinstead ofUnion[List[str], str]).
137-143: Improved error message formattingThe error message formatting has been improved with proper line breaks and trailing commas, which is consistent with modern Python style guidelines.
157-159: Consistent error message formattingThe error message formatting here follows the same pattern as other error messages in the file, maintaining consistency.
178-178: Return type annotation modernizedThe return type annotation has been correctly updated to use Python 3.10+ dictionary syntax (
dict[str, Any]instead ofDict[str, Any]).
221-221: Static method return type modernizedThe static method return type has been correctly updated to use Python 3.10+ dictionary syntax.
zmapsdk/schemas.py (1)
1-42: Well-structured Pydantic schema definitionsThe introduction of a centralized schema module is an excellent architectural improvement. The Pydantic models are well-defined with:
- Proper type annotations using Python 3.10+ syntax
- Descriptive Field annotations that will enhance API documentation
- Clear separation of concerns by moving these models out of the API implementation
This change improves modularity, makes the code more maintainable, and creates a clear contract for API requests and responses.
zmapsdk/cli.py (6)
8-8: Good migration to Click frameworkThe adoption of
clickinstead ofargparseis a good modernization choice. Click provides better composability, help text formatting, and error handling.
14-21: Well-structured logging setup functionExtracting the logging configuration into a dedicated function improves modularity and makes the code more maintainable.
23-34: Well-implemented version commandThis is a good implementation of the version command using Click's callback mechanism. The error handling is robust, ensuring that users get meaningful feedback even if ZMap isn't installed.
36-48: Well-structured command groupThe main command group is well-structured with appropriate documentation and version flag handling.
50-77: Comprehensive API command implementationThe API command implementation includes:
- Sensible defaults for host and port
- Good help text for each option
- Robust error handling with traceback for verbose mode
- Clear user feedback with startup URLs
This is a significant improvement over the previous approach.
80-83: Simplified main functionThe main function has been simplified to just invoke the Click CLI with appropriate environment variable prefix. This is cleaner and follows Click's best practices.
pyproject.toml (5)
2-3: Successfully migrated to Poetry build systemThe migration from setuptools to Poetry is a good choice for modern Python project management. Poetry offers improved dependency resolution and a better developer experience.
5-26: Well-structured Poetry project metadataThe project metadata has been properly restructured to follow Poetry's format, with all required fields and appropriate classifiers. The removal of Python 3.7-3.9 classifiers correctly aligns with the updated minimum Python version requirement.
28-36: Dependencies properly specifiedDependencies have been well-specified with appropriate version constraints. The addition of libraries like
click,tomli, andhttpxsupports the modernization efforts seen in the code changes.
38-49: Comprehensive development dependenciesThe development dependencies section is comprehensive, including tools for formatting, testing, and documentation. The specific version constraints will help ensure reproducible development environments.
43-45:✅ Verification successful
Verify pytest dependencies compatibility
Ensure that the pytest and its plugins are compatible with each other. The current versions look recent, but it's good practice to verify compatibility.
🏁 Script executed:
#!/bin/bash # Check compatibility between pytest and plugins echo "Checking pytest and plugin compatibilities..." pip install pytest==8.3.5 pytest-asyncio==0.26.0 pytest-sugar==1.0.0 python -c "import pytest, pytest_asyncio, pytest_sugar; print(f'Installed versions: pytest={pytest.__version__}, pytest-asyncio={pytest_asyncio.__version__}, pytest-sugar={pytest_sugar.__version__}')" pytest --versionLength of output: 1508
pytest dependencies compatibility confirmed
pytest (8.3.5), pytest-asyncio (0.26.0) and pytest-sugar (1.0.0) all import successfully together, confirming that these versions are compatible. No changes to the
pyproject.tomldependencies are needed.zmapsdk/parser.py (1)
8-9: Type annotations modernized to Python 3.10+ standardsThe imports now correctly use
collections.abc.Iteratorand directtyping.Anyimport, following modern Python practices.zmapsdk/api.py (3)
10-16: Good migration of schemas to a dedicated moduleMoving the Pydantic models to a separate schema module improves code organization and maintainability.
52-73: Modern type hint syntax used consistentlyUsing Python 3.10+ type hinting syntax (
list[str]instead ofList[str]) and union syntax (str | Noneinstead ofOptional[str]) makes the code more maintainable.
157-165: Improved parameter preprocessing for the scan methodThe code now properly filters out None values and the unnecessary
return_resultskey before passing parameters to the scan method.zmapsdk/core.py (4)
15-44: Excellent centralization of option categoriesCreating the
ZMapOptionCategoriesclass with frozen sets for input and output options improves maintainability and reduces the chance of errors when classifying options.
63-109: Improved modularity with cleaner separation of concernsThe refactored
scanmethod now clearly separates initialization, core parameter setting, and option distribution to the appropriate configuration objects. This makes the code more maintainable and easier to reason about.
110-133: Well-designed option processing helper methodThe new
_process_scan_optionsmethod efficiently distributes options to the appropriate configuration objects based on predefined categories, making the code more flexible and easier to maintain.
134-333: Consistent modernization of type hints and docstringsAll method signatures and docstrings have been updated to use Python 3.10+ type annotations with improved formatting, making the API clearer and more maintainable.
zmapsdk/runner.py (4)
34-39: More concise subprocess executionUsing
capture_output=Trueis more concise than specifying stdout and stderr separately.
154-154: Use join on an empty string for consistent string concatenationWhen joining string collections, using the empty string as a delimiter is more explicit and consistent.
158-163: Simplified subprocess executionUsing
capture_output=Truesimplifies the code by removing redundant parameters.
370-375: Consistent use of capture_output parameterThe subprocess calls now consistently use
capture_output=Truethroughout the codebase, improving readability and maintainability.Also applies to: 386-391
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (7)
zmapsdk/input.py (7)
40-40:⚠️ Potential issuePreserve traceback when raising exceptions
While the exception message formatting has been improved, the original traceback should be preserved using the
fromkeyword:- raise ZMapInputError(f"Invalid subnet: {subnet} - {e!s}") + raise ZMapInputError(f"Invalid subnet: {subnet} - {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
40-40: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
122-122:⚠️ Potential issuePreserve traceback when raising exceptions
Preserve the original exception chain for better debugging:
- raise ZMapInputError(f"Invalid subnet in blocklist: {subnet} - {e!s}") + raise ZMapInputError(f"Invalid subnet in blocklist: {subnet} - {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
122-122: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
129-129:⚠️ Potential issuePreserve traceback when raising exceptions
Good update to catch the more general
OSErrorinstead of the deprecatedIOError, but also preserve the exception chain:- raise ZMapInputError(f"Failed to create blocklist file: {e!s}") + raise ZMapInputError(f"Failed to create blocklist file: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
129-129: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
153-153:⚠️ Potential issuePreserve traceback when raising exceptions
Apply the same traceback preservation pattern here:
- raise ZMapInputError(f"Invalid subnet in allowlist: {subnet} - {e!s}") + raise ZMapInputError(f"Invalid subnet in allowlist: {subnet} - {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
153-153: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
160-160:⚠️ Potential issuePreserve traceback when raising exceptions
Preserve the exception chain for better debugging:
- raise ZMapInputError(f"Failed to create allowlist file: {e!s}") + raise ZMapInputError(f"Failed to create allowlist file: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
160-160: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
184-184:⚠️ Potential issuePreserve traceback when raising exceptions
Preserve the exception chain for better debugging:
- raise ZMapInputError(f"Invalid IP address: {ip} - {e!s}") + raise ZMapInputError(f"Invalid IP address: {ip} - {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
184-184: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
191-191:⚠️ Potential issuePreserve traceback when raising exceptions
Preserve the exception chain:
- raise ZMapInputError(f"Failed to create target file: {e!s}") + raise ZMapInputError(f"Failed to create target file: {e!s}") from e🧰 Tools
🪛 Ruff (0.8.2)
191-191: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🧹 Nitpick comments (2)
zmapsdk/runner.py (1)
249-252: Use contextlib.suppress for cleaner exception handlingReplace the try-except-pass pattern with the more idiomatic contextlib.suppress:
- try: - os.unlink(temp_file) - except OSError: - pass + import contextlib + with contextlib.suppress(OSError): + os.unlink(temp_file)🧰 Tools
🪛 Ruff (0.8.2)
249-252: Use
contextlib.suppress(OSError)instead oftry-except-passReplace with
contextlib.suppress(OSError)(SIM105)
zmapsdk/parser.py (1)
159-163: Use contextlib.suppress for cleaner exception handlingReplace the try-except-pass pattern with the more idiomatic contextlib.suppress:
- try: - row[key] = float(value) if "." in value else int(value) - except ValueError: - pass # Keep as string if conversion fails + import contextlib + with contextlib.suppress(ValueError): + row[key] = float(value) if "." in value else int(value)🧰 Tools
🪛 Ruff (0.8.2)
160-163: Use
contextlib.suppress(ValueError)instead oftry-except-pass(SIM105)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/workflows/release.yml(4 hunks).gitignore(1 hunks)zmapsdk/__main__.py(1 hunks)zmapsdk/cli.py(1 hunks)zmapsdk/core.py(1 hunks)zmapsdk/input.py(1 hunks)zmapsdk/output.py(1 hunks)zmapsdk/parser.py(5 hunks)zmapsdk/runner.py(12 hunks)
✅ Files skipped from review due to trivial changes (3)
- .gitignore
- zmapsdk/main.py
- .github/workflows/release.yml
🚧 Files skipped from review as they are similar to previous changes (2)
- zmapsdk/output.py
- zmapsdk/core.py
🧰 Additional context used
🧬 Code Graph Analysis (3)
zmapsdk/input.py (3)
zmapsdk/exceptions.py (1)
ZMapInputError(29-32)zmapsdk/core.py (4)
create_blocklist_file(212-223)create_allowlist_file(225-236)create_target_file(238-249)generate_standard_blocklist(251-261)zmapsdk/api.py (1)
generate_standard_blocklist(100-120)
zmapsdk/parser.py (2)
zmapsdk/exceptions.py (1)
ZMapParserError(41-44)zmapsdk/core.py (4)
parse_metadata(280-290)extract_ips(292-307)stream_results(309-320)count_results(322-332)
zmapsdk/runner.py (6)
zmapsdk/config.py (2)
ZMapScanConfig(13-170)to_dict(139-145)zmapsdk/exceptions.py (1)
ZMapCommandError(12-20)zmapsdk/input.py (2)
ZMapInput(12-243)to_dict(221-243)zmapsdk/output.py (3)
ZMapOutput(11-247)to_dict(178-218)set_output_file(31-46)zmapsdk/core.py (6)
run(134-144)scan(63-108)get_probe_modules(146-153)get_output_modules(155-162)get_output_fields(164-174)get_interfaces(176-183)zmapsdk/api.py (5)
run(196-200)get_probe_modules(53-55)get_output_modules(59-61)get_output_fields(65-67)get_interfaces(71-73)
🪛 Ruff (0.8.2)
zmapsdk/input.py
40-40: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
122-122: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
129-129: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
153-153: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
160-160: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
184-184: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
191-191: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
zmapsdk/parser.py
77-77: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
109-109: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
133-133: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
160-163: Use contextlib.suppress(ValueError) instead of try-except-pass
(SIM105)
169-169: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
192-192: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
247-247: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
282-282: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
zmapsdk/runner.py
40-40: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
172-172: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
249-252: Use contextlib.suppress(OSError) instead of try-except-pass
Replace with contextlib.suppress(OSError)
(SIM105)
🔇 Additional comments (16)
zmapsdk/input.py (2)
19-22: The type hint modernization looks goodProper use of Python 3.10+ type hint syntax with union types (e.g.,
str | Noneinstead ofOptional[str]).
221-243: Well-structured dictionary building methodThe
to_dict()method is well-structured and follows best practices by only including non-empty values.zmapsdk/cli.py (6)
14-20: Good addition of dedicated logging setupThe new
setup_loggingfunction properly configures logging with the appropriate level and format based on verbosity.
23-33: Well-implemented version display functionalityThe
print_versioncallback follows best practices for Click by checking the context'sresilient_parsingattribute and providing useful error handling if version detection fails.
36-47: Clean CLI structure using ClickGood refactoring to use Click's command group pattern, which provides a more structured and maintainable CLI architecture than the previous argparse implementation.
50-77: Improved API server command with clear error handlingThe API server command is well-structured with good error handling, including:
- Clear option definitions with helpful descriptions
- Informative output messages
- Proper error handling with conditional traceback display based on verbosity
80-82: Simplified main function with environment variable supportGood simplification of the main function to leverage Click's built-in environment variable support with the
auto_envvar_prefixparameter.
85-86: Standard main guard patternThe main guard pattern is correctly implemented, avoiding the unnecessary use of
sys.exit()since Click already handles exit codes.zmapsdk/runner.py (5)
34-38: Improved subprocess call with explicit parametersGood update to use
capture_output=Trueandcheck=Truefor clearer intent compared to the older style of subprocess calls.
78-86: Improved method signature with modern type hintsThe updated method signature uses modern Python 3.10+ type annotations, which improves code readability and static type checking.
158-164: Standardized subprocess callsGood update to standardize the subprocess call with explicit
capture_output=Trueandcheck=Falseparameters, which makes the intent clearer.
359-363: Improved imports organization in fallback codeGood organization of imports in the fallback code path, with platform-specific imports grouped together.
370-375: Consistent subprocess call patternGood consistency in updating all subprocess calls to use the same modern pattern with
capture_output=Trueandcheck=False.Also applies to: 386-391
zmapsdk/parser.py (3)
8-9: Updated imports to use modern Python conventionsGood update to import
Iteratorfromcollections.abcinstead oftyping, which aligns with modern Python import practices.
195-198: Improved method signature with modern type hintsGood update to use Python 3.10+ type annotations with the
Iteratortype properly imported fromcollections.abc.
270-271: Simplified boolean assignmentGood simplification of the code by directly assigning the boolean result of the check.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pyproject.toml (2)
5-13: Enhance project metadata with URLs
The[tool.poetry]section is well structured. To further improve discoverability, consider adding a[tool.poetry.urls]table for fields likehomepageanddocumentation, for example:[tool.poetry.urls] homepage = "https://happyhackingspace.github.io/ZmapSDK" documentation = "https://happyhackingspace.github.io/ZmapSDK/docs"
29-36: Add upper bounds to runtime dependencies
Several dependencies (e.g.,fastapi,uvicorn,netifaces) are specified only with a minimum version. It’s recommended to add<upper bounds to guard against breaking changes in future major releases. For example:-[tool.poetry.dependencies] -fastapi = ">=0.100.0" +fastapi = ">=0.100.0,<0.101.0" -uvicorn = ">=0.22.0" +uvicorn = ">=0.22.0,<0.23.0" -netifaces = ">=0.11.0" +netifaces = ">=0.11.0,<0.12.0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(1 hunks)
🔇 Additional comments (3)
pyproject.toml (3)
2-3: Switch to Poetry build backend
Migrating the build system to Poetry withpoetry-core>=1.0.0and thepoetry.core.masonry.apibackend is correct and aligns with modern Python packaging practices.
17-17: Classifier update approved
Including"Intended Audience :: System Administrators"alongside developers and IT professionals accurately reflects the target user base for a network scanning SDK.
38-49: Verify Poetry version for dev dependency groups
You’re using the new[tool.poetry.group.dev.dependencies]syntax (introduced in Poetry ≥1.2.0). Please confirm that the CI setup installs a compatible Poetry version so these groups are correctly recognized and installed.
|
pls resolve conflicts |
|
build fails |
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (2)
pyproject.toml (1)
50-50: 🛠️ Refactor suggestionDefine console‐script entry points
Since the CLI was refactored to useclick, you should expose your commands via[tool.poetry.scripts], for example:[tool.poetry.scripts] zmap = "zmapsdk.cli:main"This ensures the
zmapcommand gets installed and is discoverable.zmapsdk/api.py (1)
122-143:⚠️ Potential issuePreserve exception traceback for better debugging
The exception handling doesn't preserve the original exception chain.
- raise HTTPException(status_code=500, detail=str(e)) + raise HTTPException(status_code=500, detail=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
142-142: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
♻️ Duplicate comments (2)
zmapsdk/runner.py (2)
39-40: Preserve exception traceback for better debuggingThe exception handling doesn't preserve the original exception chain.
- raise ZMapCommandError(command=self.zmap_path, returncode=-1, stderr=str(e)) + raise ZMapCommandError(command=self.zmap_path, returncode=-1, stderr=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
40-40: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
171-172: Preserve exception traceback for better debuggingThe exception handling doesn't preserve the original exception chain.
- raise ZMapCommandError(command=" ".join(cmd), returncode=-1, stderr=str(e)) + raise ZMapCommandError(command=" ".join(cmd), returncode=-1, stderr=str(e)) from e🧰 Tools
🪛 Ruff (0.8.2)
172-172: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
🧹 Nitpick comments (3)
pyproject.toml (1)
17-17: Add missing license classifier
You’ve declaredlicense = "MIT", but there’s no corresponding classifier. Please include:"License :: OSI Approved :: MIT License",in the
classifierslist to improve discoverability on PyPI.zmapsdk/api.py (1)
67-73: Optimize dictionary iterationThe code is accessing dictionary keys inefficiently.
- return [iface for iface in psutil.net_if_addrs().keys()] + return [iface for iface in psutil.net_if_addrs()]🧰 Tools
🪛 Ruff (0.8.2)
72-72: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
zmapsdk/runner.py (1)
246-253: Use contextlib.suppress for cleaner error suppressionThe temporary file cleanup can be simplified by using
contextlib.suppress.- if temp_file and os.path.exists(temp_file): - try: - os.unlink(temp_file) - except OSError: - pass + if temp_file and os.path.exists(temp_file): + from contextlib import suppress + with suppress(OSError): + os.unlink(temp_file)🧰 Tools
🪛 Ruff (0.8.2)
249-252: Use
contextlib.suppress(OSError)instead oftry-except-passReplace with
contextlib.suppress(OSError)(SIM105)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
pyproject.toml(1 hunks)setup.py(3 hunks)zmapsdk/api.py(5 hunks)zmapsdk/runner.py(13 hunks)
✅ Files skipped from review due to trivial changes (1)
- setup.py
🧰 Additional context used
🪛 Ruff (0.8.2)
zmapsdk/api.py
7-7: fastapi.responses.FileResponse imported but unused
Remove unused import: fastapi.responses.FileResponse
(F401)
72-72: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
95-95: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
119-119: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
zmapsdk/runner.py
40-40: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
172-172: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
249-252: Use contextlib.suppress(OSError) instead of try-except-pass
Replace with contextlib.suppress(OSError)
(SIM105)
🔇 Additional comments (13)
pyproject.toml (5)
2-3: Properly configured Poetry build system
The[build-system]section correctly specifiespoetry-coreas the build backend, ensuring that Poetry handles dependency resolution and packaging.
5-13: Poetry metadata block is correct
The transition from setuptools metadata to the[tool.poetry]format is well executed. Fields such asname,version,description,readme,authors,packages, andrepositoryalign with Poetry’s conventions.
28-31: Core dependencies look good
The Python version requirement (>=3.10) and essential runtime dependencies (fastapi,uvicorn) match the project’s targets and new features.
33-36: Version bounds for new dependencies are precise
The added libraries (click,tomli,tomlkit,httpx) use sensible lower- and upper-bound constraints. Please verify these upper limits against your tested compatibility matrix.
38-49: Dev dependencies align with CI and pre-commit hooks
Your development group covers formatting (black,isort,pyupgrade), testing (pytest,pytest-asyncio,pytest-sugar), and documentation (sphinx,sphinx-autoapi, etc.), matching the new GitHub Actions workflow.zmapsdk/api.py (1)
156-164: Improved parameter handling for scan requestsThe code now properly filters out None values and the unused
return_resultskey when preparing scan parameters, which prevents passing unnecessary parameters to the ZMap instance.zmapsdk/runner.py (7)
42-76: Improved command building with cleaner formattingThe
_build_commandmethod has been enhanced to handle list-type parameters explicitly and the overall formatting has been improved for better readability.
78-86: Modernized type annotations with Python 3.10+ syntaxMethod signatures have been updated to use the more concise union syntax (
|) instead ofOptional. This is consistent with Python 3.10+ typing standards.
138-154: Enhanced real-time output processingThe code now uses
iter(process.stdout.readline, "")for iterating through output lines, which is a more idiomatic approach in Python for reading file-like objects until they're empty.
157-164: Improved subprocess handlingThe code now uses explicit parameters for subprocess execution, making the intent clearer:
capture_output=True- explicitly shows output is being capturedtext=True- ensures output is returned as a stringcheck=False- explicitly indicates that non-zero return codes won't raise an exceptionThis improves code readability and maintainability.
226-239: Improved error handling with better command constructionThe code now builds the full command string more explicitly when reporting errors, which improves debugging capabilities.
358-361: Improved imports organizationThe code now organizes imports more clearly in the fallback case for network interface detection.
369-374: Consistent subprocess execution styleThe subprocess execution style has been standardized throughout the codebase, using named parameters instead of positional parameters. This improves readability and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
38-49: Pin upper bounds for dev dependencies
To prevent unintentional breaks when dev tools release new major versions, consider adding upper bounds. For example:black = ">=25.1.0,<26.0.0" isort = ">=6.0.1,<7.0.0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml(1 hunks)
🔇 Additional comments (5)
pyproject.toml (5)
2-3: Poetry build backend correctly configured
The[build-system]table now referencespoetry-core>=1.0.0withpoetry.core.masonry.apias the build backend. This aligns with Poetry’s recommended setup.
10-13: Project metadata fields look good
Theauthors,packages, andrepositoryentries under[tool.poetry]are correctly specified, ensuring proper attribution and packaging.
17-17: Classifier addition is appropriate
Including"Intended Audience :: System Administrators"accurately reflects one of the SDK’s primary user groups.
28-37: Production dependencies correctly declared
All core dependencies have been moved to[tool.poetry.dependencies], and thepsutil = "^5.9.8"constraint fixes the previous invalid major‐version requirement.
38-49:✅ Verification successful
Verify Poetry version for
group.devsyntax
The[tool.poetry.group.dev.dependencies]format requires Poetry >=1.2. Please confirm that the CI workflow installs a compatible Poetry version.
🏁 Script executed:
#!/bin/bash # Check installed Poetry version poetry --versionLength of output: 115
🏁 Script executed:
#!/bin/bash # Search CI workflows for Poetry setup or installation steps rg -nE "actions/setup-poetry|poetry install|pip install poetry" .github/workflows || echo "No Poetry setup found in workflows"Length of output: 254
🏁 Script executed:
#!/bin/bash # List all GitHub Actions workflow files echo "Workflow files:" find .github/workflows -type f \( -name '*.yml' -o -name '*.yaml' \) # Search for any Poetry usage in workflows echo -e "\nSearching for Poetry references in workflow files..." grep -R -n 'poetry' .github/workflows || echo "No references to Poetry found" # Specifically check for setup actions or install commands echo -e "\nSearching for setup-poetry or pip install poetry..." grep -R -n -E 'actions/setup-poetry|pip install poetry' .github/workflows || echo "No setup-poetry or pip install poetry steps found"Length of output: 745
🏁 Script executed:
#!/bin/bash # Show context around the Poetry installation step in CI workflow rg -n -C5 'snok/install-poetry@v1' .github/workflows/ci.ymlLength of output: 362
No changes needed: CI installs Poetry 1.7.1 (≥ 1.2)
The CI workflow in.github/workflows/ci.ymlusessnok/install-poetry@v1withversion: 1.7.1, which fully supports the[tool.poetry.group.dev.dependencies]syntax introduced in Poetry 1.2.
major conflicts have been resolved.i also resolved the exception handling conflicts suggested by code rabbit since they were not critical |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
pyproject.toml (4)
5-13: Enhance project metadata withlicense-fileand documentation URLs
Consider addinglicense-fileto ensure the LICENSE is included in distributions, and include fields likehomepageanddocumentationunder metadata to improve discoverability on PyPI.Example additions:
license-file = "LICENSE" homepage = "https://github.com/HappyHackingSpace/ZmapSDK" documentation = "https://zmapsdk.readthedocs.io"
14-18: Review operating system classifiers
Currently onlyOperating System :: POSIX :: Linuxis listed. If the SDK supports other platforms (e.g., Windows or macOS), addOperating System :: OS Independentor the specific OS classifiers. Otherwise, confirm Linux-only support.
28-36: Pin an upper bound for the Python requirement
Lockingpython = ">=3.10"without an upper bound may inadvertently allow Python 4.x, which could break compatibility. Consider using:python = ">=3.10,<4.0"
38-49: Refine dev dependency grouping and version constraints
You might split dev dependencies into dedicated groups (e.g.,test,docs) for clearer installs (poetry install --with test). Also, consider relaxing exact pins (for example, usesphinx = "^8.0.0"instead of==8.0.0) to pick up minor fixes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml(1 hunks)
🔇 Additional comments (1)
pyproject.toml (1)
1-3: Build system configuration is correctly migrated to Poetry
The[build-system]table properly requirespoetry-core>=1.0.0and sets the Poetry core masonry API as the build backend.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
pyproject.toml (2)
12-13: 🛠️ Refactor suggestionAdd CLI entry point for the installed package
Since the CLI was refactored to useclick, add a console script under[tool.poetry.scripts]so the CLI is installed as an executable:+[tool.poetry.scripts] +zmap = "zmapsdk.cli:main"
32-32:⚠️ Potential issueInvalid
psutilversion constraint
The constraint^7.0.0refers to a major version ofpsutilthat doesn’t exist (latest stable is 5.x). Update to a valid range, for example:-psutil = "^7.0.0" +psutil = ">=5.9.0,<6.0.0"
🧹 Nitpick comments (1)
pyproject.toml (1)
46-46: Standardize Sphinx version specifier
Using==8.0.0is too restrictive and inconsistent with ranged constraints elsewhere. Consider a range to allow patch updates:-sphinx = "==8.0.0" +sphinx = ">=8.0.0,<9.0.0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
pyproject.toml(1 hunks)
🔇 Additional comments (5)
pyproject.toml (5)
2-3: Correct build system configuration
The project is correctly migrated to usepoetry-coreas the build backend per the Poetry documentation.
7-13: Project metadata updated appropriately
Version bump to0.1.4, authors formatted as a string list, package inclusion and repository URL are correctly specified under[tool.poetry].
17-17: Classifier addition looks good
The new classifier for system administrators is appropriate given the SDK’s use cases.
29-31: Dependencies section properly migrated
Python version requirement and third-party dependencies (fastapi,uvicorn,click,tomli,tomlkit,httpx) are correctly declared under[tool.poetry.dependencies].Also applies to: 33-37
38-50: Dev dependencies section review
The development dependencies are correctly grouped undergroup.dev.dependencies. No immediate issues detected here.
Summary by CodeRabbit
New Features
clicklibrary with enhanced command grouping and a global--versionflag.Refactor
Chores
.gitignoreto exclude documentation build outputs.Style
Documentation
Bug Fixes