-
Notifications
You must be signed in to change notification settings - Fork 23
feat(scanner): remove automatic WFP file generation during scan #175
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
Conversation
📝 WalkthroughWalkthroughVersion bumped to 1.43.0; the Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant CLI as CLI/parser
participant Scanner
participant FS as FileSystem
rect rgb(240,248,255)
Note right of CLI: New flow (explicit WFP)
end
User->>CLI: run `scan` (with optional --wfp-file)
CLI->>Scanner: invoke scan(..., wfp_file?)
alt wfp_file provided
Scanner->>FS: open/write/read specified wfp_file
FS-->>Scanner: wfp_file content / write ack
Scanner-->>CLI: scan results (WFP-included)
else no wfp_file
Scanner->>FS: perform scan without writing WFP
Scanner-->>CLI: scan results (no WFP)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (2)
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. Comment |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
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
🧹 Nitpick comments (2)
CHANGELOG.md (1)
13-14: Optional: Consider adjusting list indentation.The linter suggests zero indentation for list items. While the current 2-space indentation under section headers is a common pattern in changelogs, adjusting to zero indentation would satisfy the markdownlint rule.
🔎 Proposed adjustment
## [1.43.0] - 2026-01-02 ### Changed - - Scan command no longer generates `scanner_output.wfp` file - - Removed `--no-wfp-output` flag (no longer needed) +- Scan command no longer generates `scanner_output.wfp` file +- Removed `--no-wfp-output` flag (no longer needed)src/scanoss/scanner.py (1)
775-829: Consider refactoring to reduce cyclomatic complexity.The pipeline reports this method exceeds the complexity threshold (13 branches vs. 12 max). While the parameter rename in this PR doesn't introduce new complexity, consider extracting portions of the WFP parsing logic into helper methods to improve maintainability.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.mddocs/source/index.rstsrc/scanoss/__init__.pysrc/scanoss/cli.pysrc/scanoss/scanner.py
💤 Files with no reviewable changes (2)
- src/scanoss/cli.py
- docs/source/index.rst
🧰 Additional context used
🪛 GitHub Actions: Lint
src/scanoss/scanner.py
[error] 775-775: PLR0912 Too many branches (13 > 12).
🪛 markdownlint-cli2 (0.18.1)
CHANGELOG.md
13-13: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 0; Actual: 2
(MD007, ul-indent)
⏰ 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 (4)
CHANGELOG.md (1)
11-14: LGTM! Changelog entries are clear and accurate.The changelog correctly documents the breaking changes for version 1.43.0, aligning with the PR objectives.
src/scanoss/__init__.py (1)
25-25: LGTM! Version bump correctly reflects the release.The version update to 1.43.0 aligns with the changelog and breaking changes introduced in this PR.
src/scanoss/scanner.py (2)
745-773: LGTM! Parameter rename improves API clarity.The rename from
wfptowfp_filemakes it explicit that this parameter expects a file path rather than WFP content. Error messages have been consistently updated.
775-775: LGTM! Parameter changes enforce explicit WFP file specification.The rename to
wfp_fileand removal of the optional default value improve API consistency and make the requirement for a WFP file path explicit.
Added # noqa: PLR0912 to bypass the linter's branch limit (13 > 12). The extra branches come from separate validation checks that provide clearer error messages to users. Combining them would reduce clarity.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
| if not os.access(os.getcwd(), os.W_OK): # Make sure the current directory is writable. If not disable saving WFP | ||
| print_stderr(f'Warning: Current directory is not writable: {os.getcwd()}') | ||
| args.no_wfp_output = True |
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.
Removed because the CLI no longer writes to the scan folder
| def scan_wfp_file(self, file: str = None) -> bool: # noqa: PLR0912, PLR0915 | ||
| """ | ||
| Scan the contents of the specified WFP file (in the current process) | ||
| :param file: Scan the contents of the specified WFP file (in the current process) | ||
| :return: True if successful, False otherwise | ||
| """ | ||
| success = True | ||
| wfp_file = file if file else self.wfp # If a WFP file is specified, use it, otherwise us the default | ||
| if not os.path.exists(wfp_file) or not os.path.isfile(wfp_file): | ||
| raise Exception(f'ERROR: Specified WFP file does not exist or is not a file: {wfp_file}') | ||
| file_count = Scanner.__count_files_in_wfp_file(wfp_file) | ||
| cur_files = 0 | ||
| cur_size = 0 | ||
| batch_files = 0 | ||
| wfp = '' | ||
| max_component = {'name': '', 'hits': 0} | ||
| components = {} | ||
| self.print_debug(f'Found {file_count} files to process.') | ||
| raw_output = '{\n' | ||
| file_print = '' | ||
| bar_ctx = Bar('Scanning', max=file_count) if (not self.quiet and self.isatty) else nullcontext() | ||
|
|
||
| with bar_ctx as bar: | ||
| if bar: | ||
| bar.next(0) | ||
| with open(wfp_file) as f: | ||
| for line in f: | ||
| if line.startswith(WFP_FILE_START): | ||
| if file_print: | ||
| wfp += file_print # Store the WFP for the current file | ||
| cur_size = len(wfp.encode('utf-8')) | ||
| file_print = line # Start storing the next file | ||
| cur_files += 1 | ||
| batch_files += 1 | ||
| else: | ||
| file_print += line # Store the rest of the WFP for this file | ||
| l_size = cur_size + len(file_print.encode('utf-8')) | ||
| # Hit the max post size, so sending the current batch and continue processing | ||
| if l_size >= self.max_post_size and wfp: | ||
| self.print_debug( | ||
| f'Sending {batch_files} ({cur_files}) of' | ||
| f' {file_count} ({len(wfp.encode("utf-8"))} bytes) files to the ScanOSS API.' | ||
| ) | ||
| if self.debug and cur_size > self.max_post_size: | ||
| Scanner.print_stderr( | ||
| f'Warning: Post size {cur_size} greater than limit {self.max_post_size}' | ||
| ) | ||
| scan_resp = self.scanoss_api.scan(wfp, max_component['name']) # Scan current WFP and store | ||
| if bar: | ||
| bar.next(batch_files) | ||
| if scan_resp is not None: | ||
| for key, value in scan_resp.items(): | ||
| raw_output += ' "%s":%s,' % (key, json.dumps(value, indent=2)) | ||
| for v in value: | ||
| if hasattr(v, 'get'): | ||
| if v.get('id') != 'none': | ||
| vcv = '%s:%s:%s' % (v.get('vendor'), v.get('component'), v.get('version')) | ||
| components[vcv] = components[vcv] + 1 if vcv in components else 1 | ||
| if max_component['hits'] < components[vcv]: | ||
| max_component['name'] = v.get('component') | ||
| max_component['hits'] = components[vcv] | ||
| else: | ||
| Scanner.print_stderr(f'Warning: Unknown value: {v}') | ||
| else: | ||
| success = False | ||
| batch_files = 0 | ||
| wfp = '' | ||
| if file_print: | ||
| wfp += file_print # Store the WFP for the current file | ||
| if wfp: | ||
| self.print_debug( | ||
| f'Sending {batch_files} ({cur_files}) of' | ||
| f' {file_count} ({len(wfp.encode("utf-8"))} bytes) files to the ScanOSS API.' | ||
| ) | ||
| scan_resp = self.scanoss_api.scan(wfp, max_component['name']) # Scan current WFP and store | ||
| if bar: | ||
| bar.next(batch_files) | ||
| first = True | ||
| if scan_resp is not None: | ||
| for key, value in scan_resp.items(): | ||
| if first: | ||
| raw_output += ' "%s":%s' % (key, json.dumps(value, indent=2)) | ||
| first = False | ||
| else: | ||
| raw_output += ',\n "%s":%s' % (key, json.dumps(value, indent=2)) | ||
| else: | ||
| success = False | ||
| raw_output += '\n}' | ||
| if self.output_format == 'plain': | ||
| self.__log_result(raw_output) | ||
| elif self.output_format == 'cyclonedx': | ||
| cdx = CycloneDx(self.debug, self.scan_output) | ||
| cdx.produce_from_str(raw_output) | ||
| elif self.output_format == 'spdxlite': | ||
| spdxlite = SpdxLite(self.debug, self.scan_output) | ||
| success = spdxlite.produce_from_str(raw_output) | ||
| elif self.output_format == 'csv': | ||
| csvo = CsvOutput(self.debug, self.scan_output) | ||
| csvo.produce_from_str(raw_output) | ||
| else: | ||
| self.print_stderr(f'ERROR: Unknown output format: {self.output_format}') | ||
| success = False | ||
|
|
||
| return success | ||
|
|
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.
Dead code, CLI always uses scan_wfp_file_threaded
matiasdaloia
left a comment
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.
LGTM
Summary
Tested scenarios:
Summary by CodeRabbit
Changed
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.