-
Notifications
You must be signed in to change notification settings - Fork 24
[SP-2870] fix: cyclonedx format output not writing to file #130
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
[SP-2870] fix: cyclonedx format output not writing to file #130
Conversation
WalkthroughThis update addresses an issue where the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI_Command
participant Scanner
participant CycloneDx
User->>CLI_Command: Run folder-scan/container-scan with --format cyclonedx
CLI_Command->>Scanner: Initiate scan
Scanner->>CycloneDx: produce_from_json(results)
CycloneDx-->>Scanner: (success, cdx_output)
alt success
Scanner->>CLI_Command: Write cdx_output to file
else failure
Scanner->>CLI_Command: Log/Print error
end
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (2)
src/scanoss/cyclonedx.py (1)
273-289
: Inconsistent return type in produce_from_str.The method signature indicates it returns
bool
, but it callsproduce_from_json
which now returns a tuple. This will cause the tuple to be returned instead of just the boolean.Fix the return type inconsistency:
- def produce_from_str(self, json_str: str, output_file: str = None) -> bool: + def produce_from_str(self, json_str: str, output_file: str = None) -> bool: """ Produce CycloneDX output from input JSON string :param json_str: input JSON string :param output_file: Output file (optional) :return: True if successful, False otherwise """ if not json_str: self.print_stderr('ERROR: No JSON string provided to parse.') return False try: data = json.loads(json_str) except Exception as e: self.print_stderr(f'ERROR: Problem parsing input JSON: {e}') return False - return self.produce_from_json(data, output_file) + success, _ = self.produce_from_json(data, output_file) + return successsrc/scanoss/scanner.py (1)
849-849
: Assignproduce_from_str
return value tosuccess
in CycloneDX branchesThe CycloneDX method
produce_from_str
is typed to return a singlebool
, so its result must be assigned to thesuccess
variable (unlikeproduce_from_json
, which returns a(bool, json)
tuple and is properly destructured). Otherwise,success
can remain undefined or stale. Update these lines:
src/scanoss/scanner.py:849
cdx.produce_from_str(raw_output)
success = cdx.produce_from_str(raw_output)
src/scanoss/scanner.py:969
cdx.produce_from_str(raw_output)
success = cdx.produce_from_str(raw_output)
🧹 Nitpick comments (1)
src/scanoss/scanners/scanner_hfh.py (1)
203-208
: Proper handling of the new tuple return from produce_from_json.The unpacking of the tuple and error handling logic is correct. However, the function has too many return statements (7 > 6) as indicated by the linter.
Consider consolidating the return statements to improve code quality:
- success, cdx_output = cdx.produce_from_json(scan_results) - if not success: - error_msg = 'ERROR: Failed to produce CycloneDX output' - self.base.print_stderr(error_msg) - return None - return json.dumps(cdx_output, indent=2) + success, cdx_output = cdx.produce_from_json(scan_results) + if success: + return json.dumps(cdx_output, indent=2) + else: + error_msg = 'ERROR: Failed to produce CycloneDX output' + self.base.print_stderr(error_msg) + return None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
CHANGELOG.md
(2 hunks)src/scanoss/__init__.py
(1 hunks)src/scanoss/cyclonedx.py
(3 hunks)src/scanoss/scanner.py
(11 hunks)src/scanoss/scanners/container_scanner.py
(1 hunks)src/scanoss/scanners/scanner_hfh.py
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/scanoss/cyclonedx.py (2)
src/scanoss/spdxlite.py (3)
produce_from_json
(272-287)parse
(68-79)print_stderr
(55-59)src/scanoss/scanossbase.py (1)
print_stderr
(45-49)
src/scanoss/scanner.py (12)
src/scanoss/file_filters.py (1)
FileFilters
(433-743)src/scanoss/csvoutput.py (2)
CsvOutput
(33-233)produce_from_json
(178-216)src/scanoss/cyclonedx.py (2)
CycloneDx
(36-306)produce_from_json
(173-271)src/scanoss/scancodedeps.py (2)
ScancodeDeps
(32-284)produce_from_json
(85-151)src/scanoss/scanossapi.py (1)
ScanossApi
(49-285)src/scanoss/scanossbase.py (1)
ScanossBase
(28-107)src/scanoss/scanossgrpc.py (1)
ScanossGrpc
(94-677)src/scanoss/scanpostprocessor.py (1)
ScanPostProcessor
(76-289)src/scanoss/scantype.py (1)
ScanType
(28-35)src/scanoss/spdxlite.py (2)
SpdxLite
(39-697)produce_from_json
(272-287)src/scanoss/threadeddependencies.py (1)
ThreadedDependencies
(60-240)src/scanoss/threadedscanning.py (1)
ThreadedScanning
(45-234)
🪛 LanguageTool
CHANGELOG.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...yclonedxthe output was not writing to file - Fixed when running
container-scan` w...
(QB_NEW_EN_OTHER)
[grammar] ~15-~15: There might be a mistake here.
Context: ...yclonedx` the output was not writing to file ## [1.27.0] - 2025-06-30 ### Added - Add di...
(QB_NEW_EN_OTHER)
🪛 GitHub Actions: Lint
src/scanoss/scanners/scanner_hfh.py
[error] 165-165: PLR0911 Too many return statements (7 > 6) in function '_format_cyclonedx_output'.
src/scanoss/cyclonedx.py
[error] 51-51: PLR0912 Too many branches (29 > 12) in function 'parse'.
[error] 51-51: PLR0915 Too many statements (77 > 50) in function 'parse'.
[error] 60-60: F541 f-string without any placeholders. Remove extraneous f
prefix.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (15)
src/scanoss/__init__.py (1)
25-25
: Version bump looks good.The patch version increment is appropriate for a bug fix release.
CHANGELOG.md (1)
12-16
: Changelog entry accurately documents the fixes.The entry clearly describes the CycloneDX output file writing issue that was fixed for both
folder-scan
andcontainer-scan
commands.src/scanoss/scanners/scanner_hfh.py (1)
199-199
: Correct adaptation to new CycloneDx interface.The removal of the
output_file
parameter is consistent with the new usage pattern.src/scanoss/scanners/container_scanner.py (1)
439-444
: Correct implementation of the new CycloneDx interface.The changes properly handle the new tuple return from
produce_from_json
with appropriate error handling and JSON formatting.src/scanoss/cyclonedx.py (4)
25-25
: Import reorganization improves code style.Moving the
datetime
import to the top follows Python conventions.
173-184
: Method signature change addresses the core issue.The new tuple return
(bool, json)
allows callers to access both the success status and the generated CycloneDX output data, which is essential for the file writing fix.
188-188
: Consistent tuple return for failure case.The
(False, None)
return is consistent with the new interface.
271-271
: Consistent tuple return for success case.The
(True, data)
return provides both success status and the generated CycloneDX data.src/scanoss/scanner.py (7)
25-25
: New datetime import added but potentially unused.The
datetime
import was added but I don't see it used directly in the scanner.py file. Based on the relevant code context, it appears to be used in theversion_details()
method starting at line 235.
29-51
: Import organization improved for better readability.The import statements have been reorganized to group related imports together, improving code organization and readability.
58-58
: Exception handling syntax updated to modern Python style.The exception handling has been updated from
except ImportError:
toexcept (ModuleNotFoundError, ImportError):
which is more specific and catches both potential import-related exceptions.
72-72
: Linter warning suppressions added for code complexity.The
# noqa
comments suppress legitimate linter warnings for methods with high complexity (PLR0912, PLR0915) and too many parameters (PLR0913). These suppressions are appropriate for this codebase where refactoring these methods might introduce more complexity.Also applies to: 287-287, 335-335, 535-535, 606-606, 758-758
403-403
: Long line suppressions added for readability.The
# noqa: E501
comments suppress line length warnings for comment lines that provide important context and would be less readable if wrapped.Also applies to: 660-660, 669-669
487-487
: Critical fix: Adapt to new CycloneDx.produce_from_json return signature.This change correctly adapts to the updated
CycloneDx.produce_from_json
method which now returns a tuple(bool, json)
instead of just a boolean. The unpackingsuccess, _ = cdx.produce_from_json(results)
properly extracts the success flag while discarding the JSON data.This fix addresses the core issue mentioned in the PR objectives where CycloneDX format output was not writing to files properly.
512-512
: Linter warning suppression for intentional variable reassignment.The
# noqa: PLW2901
comment suppresses the pylint warning about redefining the loop variableresponse
. This is intentional behavior where the response is being transformed (deobfuscated) in place.
Summary by CodeRabbit
--format cyclonedx
option did not write output to a file as expected.