BUG-301 | Fix parsing error in locals.tf and 'config.tf` files
#303
Conversation
WalkthroughThis pull request refactors the Changes
Sequence Diagram(s)sequenceDiagram
participant C as Caller
participant F as parse_tf_file
participant H as hcl2
C->>F: Call parse_tf_file(file)
F->>file: file.read_text()
F->>H: hcl2.loads(file_content)
alt Parsing Successful
H-->>F: Parsed data
F-->>C: Return parsed data
else Parsing Error
H-->>F: UnexpectedInput Exception
F->>F: error.get_context(file_content)
F-->>C: Return detailed error message
end
Assessment against linked issues
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ 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:
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: 0
🧹 Nitpick comments (1)
leverage/_utils.py (1)
126-131: Consider preserving exception chaining for better debuggingThe static analysis tool correctly identified a potential improvement in exception handling.
- except lark.exceptions.UnexpectedInput as error: - raise ExitError( - 1, - f"Possible invalid expression in file {file.name} near line {error.line}, column {error.column}\n" - f"{error.get_context(file.read_text())}", - ) + except lark.exceptions.UnexpectedInput as error: + raise ExitError( + 1, + f"Possible invalid expression in file {file.name} near line {error.line}, column {error.column}\n" + f"{error.get_context(file.read_text())}", + ) from errorThis preserves the exception chain, which can be helpful for debugging and logging purposes while maintaining the same user-facing error message.
🧰 Tools
🪛 Ruff (0.8.2)
127-131: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
📜 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 (2)
leverage/_utils.py(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
leverage/_utils.py
127-131: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration_tests_cli_refarch (3.9.15, 1.3.5-0.2.0)
🔇 Additional comments (2)
pyproject.toml (1)
37-37: Dependency upgrade approvedThe update of
python-hcl2from 5.1.1 to 7.0.1 aligns with the PR objectives to fix parsing issues in Terraform files. This matches the implementation changes in_utils.pywhere the parsing method was changed fromhcl2.load()tohcl2.loads().leverage/_utils.py (1)
124-133: Improved error handling for Terraform parsingThe implementation change from file object parsing to string parsing with better error handling is excellent. The new approach:
- Uses
file.read_text()andhcl2.loads()instead of the previous context manager withhcl2.load(f)- Catches
lark.exceptions.UnexpectedInputspecifically- Provides detailed error messaging with line, column, and context information
This directly addresses the PR objectives to enhance error messaging and improve user experience.
🧰 Tools
🪛 Ruff (0.8.2)
127-131: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
|
To check how much of the ref arch the new version of the parser could parse this little script was run: from pathlib import Path
import hcl2
import lark
def check_parsing(directory: Path) -> None:
print(f"Directory: {directory}")
exclude_prefixes = (".", "_", "@", "config")
if tf_files := list(directory.glob("*.tf")):
for tf_file in tf_files:
try:
parsed_tf_file = hcl2.loads(tf_file.read_text())
except lark.exceptions.UnexpectedInput as error:
print(f"Possible invalid expression in file {tf_file.name} near line {error.line}, column {error.column}")
print(error.get_context(tf_file.read_text()))
subdirectories = [
item for item in directory.iterdir()
if item.is_dir()
and not item.resolve().name.startswith(exclude_prefixes)
]
for subdirectory in subdirectories:
check_parsing(subdirectory)
root = Path(".")
check_parsing(root)The results were: The error in |
Pull Request Test Coverage Report for Build 14578385646Details
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
leverage/_utils.py (1)
124-134: Enhanced error handling with detailed parsing context.The refactoring from using file objects with
hcl2.load()to string parsing withhcl2.loads()aligns well with the upgraded parser package mentioned in the PR objectives. The improved error message now provides specific line and column numbers along with contextual snippets from the file, which will significantly help users identify and fix parsing issues.Consider following the Python best practice for re-raising exceptions by using the
fromclause:try: content = file.read_text() parsed = hcl2.loads(content) except lark.exceptions.UnexpectedInput as error: raise ExitError( 1, f"Possible invalid expression in file {file.name} near line {error.line}, column {error.column}\n" f"{error.get_context(content)}", - ) + ) from error else: return parsedThis helps distinguish between errors in exception handling and actual exceptions, preserving the original traceback.
🧰 Tools
🪛 Ruff (0.8.2)
128-132: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
leverage/_utils.py(1 hunks)tests/test_modules/test_auth.py(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/test_modules/test_auth.py (1)
tests/conftest.py (1)
propagate_logs(54-56)
🪛 Ruff (0.8.2)
leverage/_utils.py
128-132: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: integration_tests_cli_refarch (3.9.15, 1.3.5-0.2.0)
🔇 Additional comments (4)
tests/test_modules/test_auth.py (4)
25-25: Improved type consistency in mocks.Changing the return value from a string to a
PosixPathobject better matches the actual type returned by the real method, improving test accuracy.
194-196: Standardized dictionary keys for consistent lookup.The test data dictionary keys are now standardized to plain filename strings, making lookups in the mock side effects more consistent and reliable.
202-207: Added Path.read_text mock to support refactored code.This new mock function correctly simulates the behavior of
Path.read_text()which is now used in the refactoredparse_tf_filefunction, ensuring tests reflect the actual code behavior.
233-235: Unified mocking strategy for file interactions.The patching approach has been improved by targeting the specific modules actually used by the code:
configupdater.parser.openinstead of built-inopen- Added
pathlib.Path.read_textto match the refactored file reading approach- Adjusted patch ordering and corresponding function arguments
This change ensures tests accurately mirror the actual file system interactions in the code, making them more reliable and less prone to false positives.
Also applies to: 248-250, 260-265, 281-293
angelofenoglio
deleted the
bug-301-fix-parsing-error-in-locals.tf-file
branch
What?
Why?
References
locals.tfFile** #301Additional context
Summary by CodeRabbit
Bug Fixes
Chores
Tests