📝 Add docstrings to fix-windows-path-handling
#1174
Conversation
Docstrings generation was requested by @subhash-0000. * #1173 (comment) The following files were modified: * `nettacker/api/engine.py` * `nettacker/core/arg_parser.py` * `nettacker/core/messages.py` * `nettacker/core/template.py` * `nettacker/core/utils/common.py`
coderabbitai
bot
requested review from
arkid15r and
securestep9
as code owners
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis pull request refactors path handling across multiple modules to use path components and file stems instead of string splitting, adds explicit UTF-8 encoding to file operations, improves error handling in the API layer, and enhances documentation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Tip ✨ Issue Enrichment is now available for GitHub issues!CodeRabbit can now help you manage issues more effectively:
Disable automatic issue enrichmentTo disable automatic issue enrichment, add the following to your issue_enrichment:
auto_enrich:
enabled: falseComment |
|
Hi, um so I checked the PR locally and the docstring changes look good. but the CI is failing on pre-commit check though, it might just need a re run or quick formatting fix. Let me know if you want me to take a look. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nettacker/core/arg_parser.py (1)
72-106: Docstring uses tab indentation and exceeds line length limits.The docstring uses tabs instead of 4-space indentation (lines 77-85), which violates coding guidelines and likely causes the ruff-format failure.
@staticmethod def load_modules(limit=-1, full_details=False): """ Load available scanning modules from the configured modules directory. - + Parameters: - limit (int): Maximum number of modules to return; -1 means no limit. If the limit is reached a "..." placeholder will be inserted. - full_details (bool): If True, include each module's parsed `info` mapping as the value; otherwise the value will be None. - + limit (int): Maximum number of modules to return; -1 means no limit. + If the limit is reached a "..." placeholder will be inserted. + full_details (bool): If True, include each module's parsed `info` mapping + as the value; otherwise the value will be None. + Returns: - dict: Mapping of module keys ("<library>_<category>") to either their `info` dict (when full_details is True) or None. Always includes an "all" entry and may include a "..." placeholder when truncated. - + dict: Mapping of module keys ("<library>_<category>") to either their `info` + dict (when full_details is True) or None. Always includes an "all" entry + and may include a "..." placeholder when truncated. + Notes: - This function also updates the module severity and description cache (all_module_severity_and_desc) for each discovered module. + This function also updates the module severity and description cache + (all_module_severity_and_desc) for each discovered module. """
🧹 Nitpick comments (3)
nettacker/core/template.py (1)
47-52: Inconsistent return type annotation style in docstring.The
openmethod usesstr:butformatusesformatted (str):. Use a consistent style.""" Render the module's YAML template by applying the instance's inputs as format placeholders. - + Returns: - formatted (str): The template string produced by calling `open()` and applying `str.format` with `self.inputs`. + str: The template string produced by calling `open()` and applying + `str.format` with `self.inputs`. """nettacker/core/messages.py (1)
22-23: Consider adding explicit UTF-8 encoding for consistency.The
load_yamlfunction opens files without explicit encoding, whileTemplateLoader.openintemplate.pynow usesencoding="utf-8". For cross-platform consistency, consider updating this as well.def load_yaml(filename): - return yaml.load(StringIO(open(filename, "r").read()), Loader=yaml.FullLoader) + return yaml.load(StringIO(open(filename, "r", encoding="utf-8").read()), Loader=yaml.FullLoader)nettacker/api/engine.py (1)
376-382: Docstring formatting could be improved.The docstring has trailing whitespace on line 378 and the blank line uses spaces. Minor formatting issues that may contribute to the ruff-format failure.
""" - Retrieve the raw scan result content for a given report id and return it as an HTTP response. - - Validates the API key on the incoming request. If the "id" parameter is missing, responds with HTTP 400; if the report cannot be retrieved, responds with HTTP 500. - + Retrieve the raw scan result content for a given report id and return it as an HTTP response. + + Validates the API key on the incoming request. If the "id" parameter is missing, + responds with HTTP 400; if the report cannot be retrieved, responds with HTTP 500. + Returns: - Response: HTTP response whose body is the scan file content, with the MIME type determined from the file extension and a `Content-Disposition` attachment filename header. + Response: HTTP response whose body is the scan file content, with the MIME type + determined from the file extension and a `Content-Disposition` attachment + filename header. """
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
nettacker/api/engine.py(3 hunks)nettacker/core/arg_parser.py(2 hunks)nettacker/core/messages.py(2 hunks)nettacker/core/template.py(1 hunks)nettacker/core/utils/common.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use 4-space indents in Python code
Limit lines to 99 characters (ruff/ruff-format/isort profile=black)
Module and file names should use lower_snake_case
Function and variable names should use lower_snake_case
Class names should use PascalCase
Constants should use UPPER_SNAKE_CASE
Keep functions small and add type hints where practical
Files:
nettacker/core/utils/common.pynettacker/core/template.pynettacker/core/arg_parser.pynettacker/core/messages.pynettacker/api/engine.py
nettacker/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add docstrings for public APIs in the nettacker package
Files:
nettacker/core/utils/common.pynettacker/core/template.pynettacker/core/arg_parser.pynettacker/core/messages.pynettacker/api/engine.py
nettacker/core/**
📄 CodeRabbit inference engine (AGENTS.md)
Place core libraries under nettacker/core/
Files:
nettacker/core/utils/common.pynettacker/core/template.pynettacker/core/arg_parser.pynettacker/core/messages.py
nettacker/api/**
📄 CodeRabbit inference engine (AGENTS.md)
Place API-related code under nettacker/api/
Files:
nettacker/api/engine.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: OWASP/Nettacker PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T19:20:58.332Z
Learning: Applies to nettacker/**/*.py : Add docstrings for public APIs in the nettacker package
📚 Learning: 2025-09-07T19:20:58.332Z
Learnt from: CR
Repo: OWASP/Nettacker PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-07T19:20:58.332Z
Learning: Applies to nettacker/**/*.py : Add docstrings for public APIs in the nettacker package
Applied to files:
nettacker/core/utils/common.py
🧬 Code graph analysis (3)
nettacker/core/template.py (1)
nettacker/config.py (1)
Config(183-187)
nettacker/core/arg_parser.py (1)
nettacker/config.py (1)
Config(183-187)
nettacker/core/messages.py (1)
nettacker/config.py (1)
Config(183-187)
🪛 GitHub Actions: CI/CD
nettacker/core/utils/common.py
[error] 1-1: pre-commit: ruff-format reformatted 5 files; hook failed. Run 'pre-commit run --all-files' locally to apply formatting fixes and re-run CI.
nettacker/core/template.py
[error] 1-1: pre-commit: ruff-format reformatted 5 files; hook failed. Run 'pre-commit run --all-files' locally to apply formatting fixes and re-run CI.
nettacker/core/arg_parser.py
[error] 1-1: pre-commit: ruff-format reformatted 5 files; hook failed. Run 'pre-commit run --all-files' locally to apply formatting fixes and re-run CI.
nettacker/core/messages.py
[error] 1-1: pre-commit: ruff-format reformatted 5 files; hook failed. Run 'pre-commit run --all-files' locally to apply formatting fixes and re-run CI.
nettacker/api/engine.py
[error] 1-1: pre-commit: ruff-format reformatted 5 files; hook failed. Run 'pre-commit run --all-files' locally to apply formatting fixes and re-run CI.
🔇 Additional comments (6)
nettacker/core/template.py (1)
43-44: Good addition of explicit UTF-8 encoding.This ensures consistent file reading behavior across platforms, especially on Windows where the default encoding may vary.
nettacker/core/messages.py (1)
27-38: Good use ofPath.stemfor cross-platform compatibility.Using
language.steminstead of string manipulation provides more robust path handling across platforms.nettacker/core/arg_parser.py (3)
43-55: Good refactor usingPath.parent.namefor graph discovery.Using
graph_library.parent.nameinstead of string splitting is more robust for cross-platform path handling.
57-70: Good use ofPath.stemfor language discovery.Using
language.stemprovides consistent cross-platform behavior for extracting language identifiers from YAML filenames.
89-92: Good refactor usingPath.stemandPath.parent.name.Using pathlib attributes instead of string manipulation provides more robust cross-platform path handling for module discovery.
nettacker/api/engine.py (1)
374-398: Good refactor usingos.path.basenamefor filename extraction.Using
os.path.basename(filename)instead of string splitting by "/" provides correct behavior on Windows where paths may use backslashes. The improved error handling with explicit HTTP status codes (400, 500) is also a good addition.
| """ | ||
| Return the contents of the YAML module file identified by this loader's name. | ||
| The name is split on underscores to derive an action (last segment) and a library (remaining segments joined with underscores); the file at Config.path.modules_dir / action / f"{library}.yaml" is opened with UTF-8 encoding and its full text is returned. | ||
| Returns: | ||
| str: The raw text of the YAML file. | ||
| """ |
There was a problem hiding this comment.
Docstring line exceeds 99-character limit.
Line 34 is too long and likely contributing to the ruff-format failure. Split the description across multiple lines.
def open(self):
"""
Return the contents of the YAML module file identified by this loader's name.
-
- The name is split on underscores to derive an action (last segment) and a library (remaining segments joined with underscores); the file at Config.path.modules_dir / action / f"{library}.yaml" is opened with UTF-8 encoding and its full text is returned.
-
+
+ The name is split on underscores to derive an action (last segment) and a library
+ (remaining segments joined with underscores). The file at
+ Config.path.modules_dir / action / f"{library}.yaml" is opened with UTF-8 encoding
+ and its full text is returned.
+
Returns:
str: The raw text of the YAML file.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """ | |
| Return the contents of the YAML module file identified by this loader's name. | |
| The name is split on underscores to derive an action (last segment) and a library (remaining segments joined with underscores); the file at Config.path.modules_dir / action / f"{library}.yaml" is opened with UTF-8 encoding and its full text is returned. | |
| Returns: | |
| str: The raw text of the YAML file. | |
| """ | |
| """ | |
| Return the contents of the YAML module file identified by this loader's name. | |
| The name is split on underscores to derive an action (last segment) and a library | |
| (remaining segments joined with underscores). The file at | |
| Config.path.modules_dir / action / f"{library}.yaml" is opened with UTF-8 encoding | |
| and its full text is returned. | |
| Returns: | |
| str: The raw text of the YAML file. | |
| """ |
🤖 Prompt for AI Agents
In nettacker/core/template.py around lines 31 to 38, the docstring contains a
single description line that exceeds the 99-character limit (line 34); break
that long sentence into two or more shorter lines (each <=99 chars) within the
docstring, keeping existing indentation and punctuation, so the content remains
the same but wrapped across multiple lines to satisfy the formatter.
| def re_address_repeaters_key_name(key_name): | ||
| # Note: This uses "/" as a key delimiter, not a file path separator | ||
| """ | ||
| Format a slash-delimited key path into a sequence of dictionary-access segments excluding the final key. | ||
| Parameters: | ||
| key_name (str): A "/"-delimited key path ("/" is a key delimiter, not a filesystem separator). | ||
| Returns: | ||
| str: Concatenated "['key']" segments for every component of `key_name` except the last; returns an empty string if there are no components before the final segment. | ||
| """ | ||
| return "".join(["['" + _key + "']" for _key in key_name.split("/")[:-1]]) |
There was a problem hiding this comment.
Docstring uses tab indentation instead of 4 spaces.
The docstring at lines 172-176 appears to use tab indentation, which violates the coding guideline requiring 4-space indents. This is likely contributing to the ruff-format failure.
def re_address_repeaters_key_name(key_name):
# Note: This uses "/" as a key delimiter, not a file path separator
"""
Format a slash-delimited key path into a sequence of dictionary-access segments excluding the final key.
-
+
Parameters:
- key_name (str): A "/"-delimited key path ("/" is a key delimiter, not a filesystem separator).
-
+ key_name (str): A "/"-delimited key path ("/" is a key delimiter, not a filesystem separator).
+
Returns:
- str: Concatenated "['key']" segments for every component of `key_name` except the last; returns an empty string if there are no components before the final segment.
+ str: Concatenated "['key']" segments for every component of `key_name` except
+ the last; returns an empty string if there are no components before the
+ final segment.
"""📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def re_address_repeaters_key_name(key_name): | |
| # Note: This uses "/" as a key delimiter, not a file path separator | |
| """ | |
| Format a slash-delimited key path into a sequence of dictionary-access segments excluding the final key. | |
| Parameters: | |
| key_name (str): A "/"-delimited key path ("/" is a key delimiter, not a filesystem separator). | |
| Returns: | |
| str: Concatenated "['key']" segments for every component of `key_name` except the last; returns an empty string if there are no components before the final segment. | |
| """ | |
| return "".join(["['" + _key + "']" for _key in key_name.split("/")[:-1]]) | |
| def re_address_repeaters_key_name(key_name): | |
| # Note: This uses "/" as a key delimiter, not a file path separator | |
| """ | |
| Format a slash-delimited key path into a sequence of dictionary-access segments excluding the final key. | |
| Parameters: | |
| key_name (str): A "/"-delimited key path ("/" is a key delimiter, not a filesystem separator). | |
| Returns: | |
| str: Concatenated "['key']" segments for every component of `key_name` except | |
| the last; returns an empty string if there are no components before the | |
| final segment. | |
| """ | |
| return "".join(["['" + _key + "']" for _key in key_name.split("/")[:-1]]) |
🤖 Prompt for AI Agents
In nettacker/core/utils/common.py around lines 167 to 178, the docstring lines
(172–176) use tab indentation instead of the project-required 4-space
indentation; update the docstring so all interior lines are indented with 4
spaces (replace tabs with 4 spaces), maintain the existing content and wrapping,
then run the formatter/linter to verify the ruff-format failure is resolved.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Thanks, this makes sense. These look like straightforward formatting fixes (exceeding line length + indentation). If you’d like, I can open a small follow-up PR with these changes to unblock CI. |
|
@deekshithaby |
|
Thanks for the quick update! I don't have permissions to re-run the CI checks on this repository, so I can't trigger the jobs myself. Once they're re-run from your side, I'm happy to verify the results! |
|
@deekshithaby Thanks! Just to clarify - this PR #1174 was created by CodeRabbit. My actual Windows path handling fixes are in PR #1173. That PR is currently awaiting workflow approval from a maintainer to run CI checks. If you'd like to review the changes, please check out #1173 instead |
Docstrings generation was requested by @subhash-0000.
The following files were modified:
nettacker/api/engine.pynettacker/core/arg_parser.pynettacker/core/messages.pynettacker/core/template.pynettacker/core/utils/common.pyℹ️ Note