[SP-3346] feat: allow REST on decoration services #153
Conversation
WalkthroughCentralizes REST/gRPC routing in ScanossGrpc with per-call use_grpc, moves CLI --grpc to global scope, adds many new gRPC methods (cryptography, geoprovenance, semgrep), updates numerous generated protobuf descriptor option blobs/offsets, extends scanning HFHRequest, and shifts component request shapes across Components/Cryptography. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CLI as CLI
participant Comp as Components
participant SG as ScanossGrpc
participant RPC as gRPC Service
participant REST as REST API
CLI->>Comp: invoke command (use_grpc=args.grpc)
Comp->>SG: _call_api(name, payload, use_grpc=self.use_grpc)
alt use_grpc == True
SG->>RPC: _call_rpc(request)
RPC-->>SG: response
else use_grpc == False
SG->>REST: _call_rest(endpoint, payload)
REST-->>SG: response
end
SG-->>Comp: parsed dict/None
Comp-->>CLI: output/result
sequenceDiagram
autonumber
participant Client as CryptographyStub
participant Server as CryptographyServicer
note over Client,Server: New component-centric RPCs added
Client->>Server: GetComponentAlgorithmsInRange(ComponentsRequest)
Server-->>Client: ComponentsAlgorithmsInRangeResponse
Client->>Server: GetComponentsEncryptionHints(ComponentsRequest)
Server-->>Client: ComponentsEncryptionHintsResponse
sequenceDiagram
autonumber
participant Client as ComponentsClient
participant Service as Components gRPC
note over Client,Service: GetComponentStatistics now uses ComponentsRequest / ComponentsStatisticResponse
Client->>Service: GetComponentStatistics(ComponentsRequest)
Service-->>Client: ComponentsStatisticResponse
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
src/scanoss/cli.py (2)
1103-1105: Use equality/membership on tuple, not string containment for subparser check.
args.subparser in 'inspect'is a substring test and is brittle. Use a tuple membership to match aliases explicitly.Apply this diff:
- elif (args.subparser in 'inspect') and (args.subparsercmd in ('raw', 'dt')) and (args.subparser_subcmd is None): + elif (args.subparser in ('inspect', 'insp', 'ins')) and (args.subparsercmd in ('raw', 'dt')) and (args.subparser_subcmd is None): parser.parse_args([args.subparser, args.subparsercmd, '--help']) # Force utils helps to be displayed sys.exit(1)
971-979: Typo in env var name: "grcp_proxy" → "grpc_proxy".The help mentions env var "grcp_proxy"; correct name should be "grpc_proxy".
Apply this diff:
- 'Can also use the environment variable "HTTPS_PROXY=<ip>:<port>" ' - 'and "grcp_proxy=<ip>:<port>" for gRPC', + 'Can also use the environment variable "HTTPS_PROXY=<ip>:<port>" ' + 'and "grpc_proxy=<ip>:<port>" for gRPC', @@ - 'Can also use the environment variable "grcp_proxy=<ip>:<port>"', + 'Can also use the environment variable "grpc_proxy=<ip>:<port>"',Also applies to: 1009-1013
src/scanoss/api/licenses/v2/scanoss_licenses_pb2.py (1)
1-7: Pin protobuf>=6.31.0 in packaging
Updatesetup.cfgandrequirements.txtto requireprotobuf>=6.31.0so the runtime dependency matches the generated code’s Protobuf Python Version.src/scanoss/api/components/v2/scanoss_components_pb2.py (1)
1-7: Pin packaging dependencies to generated code versions
In setup.cfg (l.32–33) and requirements.txt (l.5–6), require grpcio>=1.73.1 and protobuf>=6.31.0 to match GRPC_GENERATED_VERSION in the *_pb2_grpc.py stubs and the Protobuf Python Version in *_pb2.py.src/scanoss/components.py (1)
314-321: Forwarduse_grpcoverride to_call_api
Adduse_grpc=use_grpcas the final argument inget_component_versions_jsonto honor the passed flag.--- a/src/scanoss/scanossgrpc.py +++ b/src/scanoss/scanossgrpc.py @@ def get_component_versions_json(self, search: dict, use_grpc: bool = None) -> dict: - return self._call_api( - 'components.GetComponentVersions', - self.comp_search_stub.GetComponentVersions, - search, - CompVersionRequest, - 'Sending component version data for decoration (rqId: {rqId})...', - ) + return self._call_api( + 'components.GetComponentVersions', + self.comp_search_stub.GetComponentVersions, + search, + CompVersionRequest, + 'Sending component version data for decoration (rqId: {rqId})...', + use_grpc=use_grpc, + )src/scanoss/cryptography.py (2)
187-208: ComponentsRequest builds a 'requirement' for all entries, even when no version is presentFor inputs without '@', requirement currently becomes the entire purl string, which is incorrect and may confuse the service. Only include requirement when a version is explicitly provided (or when with_range is enabled and validated).
- def _build_components_request( - self, - ) -> Optional[dict]: + def _build_components_request(self) -> Optional[dict]: """ - Load the specified purls from a JSON file or a list of PURLs and return a dictionary + Build ComponentsRequest from configured PURLs. @@ - Returns: - Optional[dict]: The dictionary containing the PURLs + Returns: + Optional[dict]: {'components': [{ 'purl': str, 'requirement'?: str }]} or None if no PURLs """ - return { - 'components': [ - { - 'purl': self._remove_version_from_purl(purl), - 'requirement': self._extract_version_from_purl(purl), - } - for purl in self.config.purl - ] - } + components = [] + for raw in (self.config.purl or []): + name, sep, ver = raw.partition('@') + item = {'purl': name} + if ver: + item['requirement'] = ver + elif self.config.with_range: + # __post_init__ should have validated; treat as defensive check + raise ScanossCryptographyError(f'Invalid PURL format: "{raw}". It must include a version (e.g., pkg:type/name@version)') + components.append(item) + return {'components': components} if components else None
224-241: _extract_version_from_purl returns the full PURL when '@' is absentThis silently produces bad 'requirement' values. Return an empty string (or None) when no version is present; also fix docstring to match behavior.
- def _extract_version_from_purl(self, purl: str) -> str: + def _extract_version_from_purl(self, purl: str) -> str: @@ - Returns: - str: The extracted version - - Raises: - ScanossCryptographyError: If the purl is not in the correct format + Returns: + str: The extracted version ('' if not present) @@ - try: - return purl.split('@')[-1] - except IndexError: - raise ScanossCryptographyError(f'Invalid purl format: {purl}') + return purl.split('@', 1)[1] if '@' in purl else ''If you adopt the _build_components_request change above, this method will only be used when '@' exists.
src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (1)
9-26: Bump grpcio to ≥1.73.1 in dependency constraints
Updaterequirements.txtandsetup.cfgto requiregrpcio>=1.73.1so the generated gRPC code’s runtime guard won’t raise at import.
🧹 Nitpick comments (16)
src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2.py (1)
30-31: Typos in OpenAPI example payloads (docs only).
- "NDV" should be "NVD".
- Keys "requirement=" / "version=" include stray '='.
These live in option examples; fix in the .proto to avoid confusing API consumers.I can submit a small follow-up PR to correct these examples if helpful.
src/scanoss/components.py (3)
94-106: Back-compat: accept JSON with “purls” when field='components'.Many users likely have existing PurlRequest JSON. When loading components from a file, transparently map a top-level "purls" array to "components" to avoid breaking existing workflows.
Apply inside load_purls after parsing the JSON:
@@ def load_purls(self, json_file: Optional[str] = None, purls: Optional[List[str]] = None, field: str = 'purls') -> Optional[dict]: - with open(json_file, 'r') as f: + with open(json_file, 'r') as f: try: - purl_request = json.loads(f.read()) + purl_request = json.loads(f.read()) + # Back-compat: allow "purls" in file when caller expects "components" + if field != 'purls' and field not in purl_request and 'purls' in purl_request: + purl_request = {field: purl_request['purls']} except Exception as e: self.print_stderr(f'ERROR: Problem parsing input JSON: {e}') return NoneAlso applies to: 115-141
189-206: Typing nits: avoid bare [] defaults; use Optional[List[str]].Signatures currently use
purls: [] = None, which loses type info. PreferOptional[List[str]]for clarity and tooling.- def get_vulnerabilities(self, json_file: str = None, purls: [] = None, output_file: str = None) -> bool: + def get_vulnerabilities(self, json_file: str = None, purls: Optional[List[str]] = None, output_file: str = None) -> bool: - def get_semgrep_details(self, json_file: str = None, purls: [] = None, output_file: str = None) -> bool: + def get_semgrep_details(self, json_file: str = None, purls: Optional[List[str]] = None, output_file: str = None) -> bool: - def get_provenance_details(self, json_file: str = None, purls: [] = None, output_file: str = None, origin: bool = False) -> bool: + def get_provenance_details(self, json_file: str = None, purls: Optional[List[str]] = None, output_file: str = None, origin: bool = False) -> bool:Also applies to: 215-233, 327-354
255-263: Docs nit: fix package examples.Small typos in docstring: duplicate "maven", and "npn" should be "npm". Harmless, but easy polish.
- :param package: Package (purl type) to search for. i.e. github/maven/maven/npn/all - default github + :param package: Package (purl type) to search for, e.g. github/maven/npm/all (default: github)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2.py (1)
38-45: Large embedded OpenAPI example blobs.These improve docs but increase wheel size. If package size matters, consider trimming or moving examples to external docs while keeping concise examples in descriptors.
src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2.py (1)
37-37: Make OpenAPI host configurable or remove default
The generated protos embedhost = "api.scanoss.com"in the OpenAPI swagger options, which forces all clients to target that endpoint. Remove the host setting from your.protofiles (or conditionally inject it via a build flag or plugin parameter) so consumers can override the server URL for local testing or self-hosted deployments.src/scanoss/api/scanning/v2/scanoss_scanning_pb2.py (1)
30-31: Add CLI flags and tests for new HFHRequest fields
No references torecursive_thresholdormin_accepted_scoreexist beyond the protobuf descriptor. Update the CLI to accept and validate these floats (e.g. enforce a 0.0–1.0 range with sane defaults) and add unit/integration tests covering unset, in-range, and out-of-range values. Confirm downstream services correctly handle defaults or missing values.src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (1)
74-82: Mark deprecated RPC with runtime warning for client awarenessConsider emitting a DeprecationWarning (or grpc status detail hint) when GetIssues is called to guide clients to the new RPCs. Keeps server logs cleaner and accelerates migration.
def GetIssues(self, request, context): - """[DEPRECATED] Get potential security issues associated with a list of PURLs + """[DEPRECATED] Get potential security issues associated with a list of PURLs This method accepts PURL-based requests and is deprecated in favor of GetComponentsIssues which accepts ComponentsRequest for better component identification """ + warnings.warn("Semgrep.GetIssues is deprecated; use GetComponentsIssues/GetComponentIssues", + DeprecationWarning, stacklevel=2) context.set_code(grpc.StatusCode.UNIMPLEMENTED)src/scanoss/cryptography.py (3)
70-81: Transport toggle default may unintentionally force RESTuse_grpc defaults to False and is always forwarded, overriding any client-side default. Suggest making it Optional[bool] with default None, and only pass when user explicitly sets --grpc.
-@dataclass -class CryptographyConfig: +@dataclass +class CryptographyConfig: @@ - use_grpc: bool = False + use_grpc: Optional[bool] = Nonedef create_cryptography_config_from_args(args) -> CryptographyConfig: return CryptographyConfig( @@ - use_grpc=getattr(args, 'grpc', False), + use_grpc=getattr(args, 'grpc', None),And then call client APIs as you do (they already accept None to use instance default, per docstring in scanossgrpc.py). Based on learnings.
40-41: Minor: grammar/spacing in error messageAdd a missing space after the period.
- f'Invalid PURL format: "{purl}".It must include a version (e.g., pkg:type/name@version)' + f'Invalid PURL format: "{purl}". It must include a version (e.g., pkg:type/name@version)'
187-198: Docstring driftThe _build_components_request docstring still mentions json_file/purls params that no longer exist. Update for clarity.
- """ - Load the specified purls from a JSON file or a list of PURLs and return a dictionary - - Args: - json_file (Optional[str], optional): The JSON file containing the PURLs. Defaults to None. - purls (Optional[List[str]], optional): The list of PURLs. Defaults to None. - - Returns: - Optional[dict]: The dictionary containing the PURLs - """ + """ + Build a ComponentsRequest from the configured PURLs. + + Returns: + Optional[dict]: {'components': [{'purl': str, 'requirement'?: str}], ...} or None if empty. + """src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2.py (1)
30-35: Pin and document Protobuf and gRPC toolchain versions
Your packaging already pins protobuf > 3.19.1 and grpcio > 1.42.0; confirm these satisfy the generated PB2 file’s requirements and record the exact protoc and gRPC-Python plugin versions (e.g. in dev-requirements.txt) for reproducible builds.src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (4)
4-4: Remove unused import.
warningsis imported but not used. Safe to drop.-import warnings
258-270: Docstring fixes: legacy flags and incorrect “See/Use” references.Several method docs mislabel endpoints as legacy and/or point to the wrong replacements or anchors. Suggested minimal fixes below.
@@ def GetHintsInRange(...): - Legacy method ... Use ComponentHintsInRange or ComponentsHintsInRange instead. + Legacy method ... Use GetComponentHintsInRange or GetComponentsHintsInRange instead. @@ def GetComponentHintsInRange(...): - Get cryptographic hints across version ranges - legacy endpoint. - - Legacy method ... + Get cryptographic hints for a single component across specified version ranges. @@ - SDKs and frameworks across version ranges. Use ComponentHintsInRange or ComponentsHintsInRange instead. + SDKs and frameworks across version ranges. + See: https://github.com/scanoss/papi/blob/main/protobuf/scanoss/api/cryptography/v2/README.md#getcomponenthintsinrange @@ def GetComponentsHintsInRange(...): - Get cryptographic hints across version ranges - legacy endpoint. - Legacy method ... + Get cryptographic hints for multiple components across specified version ranges. @@ - SDKs and frameworks across version ranges. Use ComponentHintsInRange or ComponentsHintsInRange instead. + SDKs and frameworks across version ranges. + See: https://github.com/scanoss/papi/blob/main/protobuf/scanoss/api/cryptography/v2/README.md#getcomponentshintsinrange @@ def GetEncryptionHints(...): - Legacy method ... Use ComponentHintsInRange or ComponentsHintsInRange instead. + Legacy method ... Use GetComponentEncryptionHints or GetComponentsEncryptionHints instead. @@ def GetComponentEncryptionHints(...): - See: .../README.md#componenthintsinrange + See: .../README.md#getcomponentencryptionhints @@ def GetComponentsEncryptionHints(...): - See: .../README.md#componentshintsinrange + See: .../README.md#getcomponentsencryptionhintsAlso applies to: 271-281, 282-291, 293-305, 306-318, 319-331
219-231: Docstring: add “Get” prefix to recommended methods.Current text references “ComponentVersionsInRange/ComponentsVersionsInRange”. Use full method names.
- Use ComponentVersionsInRange or ComponentsVersionsInRange instead. + Use GetComponentVersionsInRange or GetComponentsVersionsInRange instead.
161-162: Pin README links to a stable commit/tag to avoid drift.All “See:” links target main; these anchors can change. Prefer a tagged release or commit SHA.
Also applies to: 174-175, 200-201, 213-214, 239-240, 252-253, 313-314, 326-327
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
Dockerfile(1 hunks)src/scanoss/api/common/v2/scanoss_common_pb2.py(3 hunks)src/scanoss/api/components/v2/scanoss_components_pb2.py(1 hunks)src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py(3 hunks)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2.py(1 hunks)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py(9 hunks)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2.py(1 hunks)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py(1 hunks)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2.py(1 hunks)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py(5 hunks)src/scanoss/api/licenses/v2/scanoss_licenses_pb2.py(3 hunks)src/scanoss/api/scanning/v2/scanoss_scanning_pb2.py(2 hunks)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2.py(1 hunks)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py(5 hunks)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2.py(3 hunks)src/scanoss/cli.py(24 hunks)src/scanoss/components.py(8 hunks)src/scanoss/cryptography.py(8 hunks)src/scanoss/scanossgrpc.py(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/scanoss/cli.py (2)
src/scanoss/inspection/dependency_track/project_violation.py (1)
DependencyTrackProjectViolationPolicyCheck(121-478)src/scanoss/export/dependency_track.py (1)
upload_sbom_file(131-156)
src/scanoss/components.py (2)
src/scanoss/scanossgrpc.py (6)
get_vulnerabilities_json(338-356)get_semgrep_json(358-376)search_components_json(378-394)get_component_versions_json(396-411)get_provenance_origin(566-583)get_provenance_json(546-564)src/scanoss/scanossbase.py (1)
print_msg(51-56)
src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (1)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (2)
Echo(67-72)Echo(132-156)
src/scanoss/cryptography.py (2)
src/scanoss/cli.py (1)
print_stderr(88-92)src/scanoss/scanossgrpc.py (5)
get_crypto_algorithms_in_range_for_purl(605-623)get_crypto_algorithms_for_purl(585-603)get_encryption_hints_in_range_for_purl(645-663)get_encryption_hints_for_purl(625-643)get_versions_in_range_for_purl(665-683)
src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (2)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (2)
Echo(67-72)Echo(132-156)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (2)
Echo(82-88)Echo(220-244)
src/scanoss/scanossgrpc.py (7)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (4)
SearchComponents(74-79)SearchComponents(159-183)GetComponentVersions(81-86)GetComponentVersions(186-210)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (10)
GetComponentsAlgorithms(167-178)GetComponentsAlgorithms(513-537)GetComponentsAlgorithmsInRange(206-217)GetComponentsAlgorithmsInRange(594-618)GetComponentsEncryptionHints(319-330)GetComponentsEncryptionHints(837-861)GetComponentsHintsInRange(282-291)GetComponentsHintsInRange(756-780)GetComponentsVersionsInRange(245-256)GetComponentsVersionsInRange(675-699)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (2)
GetDependencies(69-75)GetDependencies(143-167)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (4)
GetCountryContributorsByComponents(98-105)GetCountryContributorsByComponents(249-273)GetOriginByComponents(125-132)GetOriginByComponents(330-354)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (2)
GetComponentsIssues(83-90)GetComponentsIssues(192-216)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (2)
GetComponentsVulnerabilities(154-166)GetComponentsVulnerabilities(382-406)src/scanoss/scanossbase.py (3)
print_stderr(45-49)print_trace(65-70)print_debug(58-63)
⏰ 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 (19)
Dockerfile (1)
32-32: requirements-dev.txt omits click; uninstall step won’t remove pinned click
requirements-dev.txt doesn’t includeclick, so thepip3 uninstall -r requirements-dev.txtstep won’t drop your pin at 8.2.1.src/scanoss/cli.py (2)
1819-1831: DT export wiring looks good; pass-through arguments match exporter signature.
upload_sbom_file(input, project_id, project_name, project_version, output)aligns with exporter expectations.
1038-1052: Approve gRPC CLI integration: Verified that all targeted subparsers include the optional--grpcflag and thatuse_grpc=args.grpcis correctly propagated to all Scanner/Components.src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (1)
69-72: Clear deprecation note for GetDependencies.Docstring deprecation to use
/v2/licenses/componentsis appropriate and non-breaking.src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2.py (1)
52-65: Verify HTTP annotations in proto definitions
I did not locate your.protosources—please confirm that bothGetComponentCpesandGetComponentVulnerabilitiesRPCs includebody: "*"in theirgoogle.api.httpoptions (e.g.post: "/v2/vulnerabilities/cpes/component" body: "*").src/scanoss/components.py (1)
76-92: Good: sane defaultuse_grpc=Falsein ScanossGrpc
The constructor signature (use_grpc: bool = False) already provides a sensible default, so no additional instance-level defaults are required.src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2.py (3)
1-7: Verify ProtoBuf generated code policy
The committedscanoss_cryptography_pb2.pyis generated code—confirm whether the project intends to check in.pb2files. If not, exclude them from version control and generate at build time.
30-31: Remove redundant Protobuf runtime pin recommendation
protobuf>3.19.1is already declared in requirements.txt and setup.cfg, covering this generated module—no additional pin needed.Likely an incorrect or invalid review comment.
61-91: Verify REST bindings and CLI coverage for cryptography endpoints.
- Old
/api/v2/cryptographypaths should remain aliased or documented as deprecated.- Ensure CLI/SDK exposes all new cryptography RPCs (component-level, list-level, range, hints, encryption) when
use_grpc=False.- (Optional) For read-only operations, consider switching from POST with
body: "*"to GET with query parameters.src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2.py (1)
47-51: Confirm HTTP path and backward-compatibility
Insrc/scanoss/api/dependencies/v2/scanoss_dependencies_pb2.py(line 49), the generated HTTP binding for GetDependencies is/v2/dependencies/dependencies. Please verify the source.protoHTTP annotation to ensure the duplicated “dependencies” segment is intentional; if not, consider changing it to/v2/dependenciesor a more specific resource path (e.g./v2/dependencies/files). Also confirm backward-compatibility for any existing/api/v2/...routes.src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (3)
114-123: Server handlers registered for new RPCsRegistration looks correct and consistent with the stub signatures. LGTM.
191-244: Static experimental client methods for new RPCsClient-side helpers mirror stub wiring and options. LGTM.
50-59: Approve: RPC stubs correct Confirmed that both ComponentsIssueResponse and ComponentIssueResponse are defined in scanoss_semgrep_pb2.src/scanoss/cryptography.py (1)
21-27: Discard incorrect suggestion: ‘header’ config is in use
Theheaderattribute in src/scanoss/cryptography.py is passed through viagetattr(args, 'header', None)(line 73) and handled by the CLI’s header-processing logic, so it is not dead code.Likely an incorrect or invalid review comment.
src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2.py (2)
51-63: Verify HTTP annotations for Geoprovenance protos
Manual check required: ensure each RPC in your .proto definitions has the correct(google.api.http)verb and request-body marker (e.g.:*where needed) and add end-to-end REST tests for Echo, GetComponentContributors, GetCountryContributorsByComponents, GetCountryContributorsByComponent, GetComponentOrigin, GetOriginByComponents, and GetOriginByComponent.
38-49: Verify OpenAPI v2 annotations importability
The verification script yielded no results—please confirm thatprotoc_gen_openapiv2.options.annotations_pb2is included in your Python environment (vendored or installed) and importable at runtime.src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (3)
53-62: [runscripts]#!/bin/bash rg -nP "unary_unary\(" -C2 src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py
345-354: Approve code changes: server registration matches all RPC stubs. All grpc.unary_unary_rpc_method_handler entries have corresponding stub methods.
485-537: Approve: experimental static wrappers match the stub and message types. Method signatures, RPC paths, and (de)serializers align with the generated stub and runtime pb2 builder—no issues.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/scanoss/scanossgrpc.py (1)
829-838: Critical: Deleting status key breaks downstream aggregation.Line 836 deletes the
statuskey before returning the response. This breaks downstream aggregation logic likeget_dependencies_json()(line 311), which expects each per-file response to carry status metadata so it can surface warnings/failures. Without the status key, aKeyErrorwill occur when trying to checkresponse['status']['status'].Remove the
del response['status']statement to preserve status for downstream callers:if response and 'status' in response: status_dict = response.get('status', {}) if status_dict.get('status') != 'SUCCESS': self.print_stderr(f'Request failed (rqId: {request_id}): {status_dict.get("message", "Unknown error")}') if status_dict.get('status') in ['FAILED', 'FAILED_WITH_WARNINGS']: return None - del response['status'] return response
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/scanoss/cryptography.py(8 hunks)src/scanoss/scanossgrpc.py(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/scanoss/scanossgrpc.py (7)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (6)
Echo(67-72)Echo(132-156)SearchComponents(74-79)SearchComponents(159-183)GetComponentVersions(81-86)GetComponentVersions(186-210)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (4)
Echo(62-67)Echo(116-140)GetDependencies(69-75)GetDependencies(143-167)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (6)
Echo(82-87)Echo(195-219)GetCountryContributorsByComponents(98-105)GetCountryContributorsByComponents(249-273)GetOriginByComponents(125-132)GetOriginByComponents(330-354)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (12)
Echo(133-139)Echo(432-456)GetComponentsAlgorithms(167-178)GetComponentsAlgorithms(513-537)GetComponentsAlgorithmsInRange(206-217)GetComponentsAlgorithmsInRange(594-618)GetComponentsEncryptionHints(319-330)GetComponentsEncryptionHints(837-861)GetComponentsHintsInRange(282-291)GetComponentsHintsInRange(756-780)GetComponentsVersionsInRange(245-256)GetComponentsVersionsInRange(675-699)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (4)
Echo(67-72)Echo(138-162)GetComponentsIssues(83-90)GetComponentsIssues(192-216)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (4)
Echo(82-88)Echo(220-244)GetComponentsVulnerabilities(154-166)GetComponentsVulnerabilities(382-406)src/scanoss/scanossbase.py (3)
print_stderr(45-49)print_trace(65-70)print_debug(58-63)
src/scanoss/cryptography.py (2)
src/scanoss/cli.py (1)
print_stderr(88-92)src/scanoss/scanossgrpc.py (5)
get_crypto_algorithms_in_range_for_purl(606-624)get_crypto_algorithms_for_purl(586-604)get_encryption_hints_in_range_for_purl(646-664)get_encryption_hints_for_purl(626-644)get_versions_in_range_for_purl(666-684)
⏰ 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 (18)
src/scanoss/cryptography.py (7)
18-29: LGTM! Configuration fields correctly extended.The addition of
debug,header, anduse_grpcfields toCryptographyConfigis appropriate and supports the unified REST/gRPC transport layer introduced in this PR.
70-81: LGTM! Configuration mapping correctly updated.The mapping of
headeranduse_grpcfrom command-line arguments is correct and aligns with the extendedCryptographyConfigdataclass.
117-117: LGTM! Correctly initializes components request.The initialization of
self.components_requestaligns with the refactored internal request structure that now uses a components-based payload.
187-208: LGTM! Request structure correctly refactored.The method rename to
_build_components_requestand the updated return structure with a'components'key align with theComponentsRequestprotobuf schema used by the gRPC client. Each component correctly includes'purl'(without version) and'requirement'(version/range).
128-137: LGTM! Empty input validation correctly added and use_grpc propagated.Line 128 now guards against both missing and empty
componentslists, addressing the previous review concern about empty input not being rejected. Theuse_grpcflag is correctly propagated to the client calls.
151-161: LGTM! Empty input validation and use_grpc propagation correct.Line 151 guards against empty payloads, and the
use_grpcflag is correctly passed to client methods.
175-181: LGTM! Empty input validation and use_grpc propagation correct.Line 175 guards against empty payloads, and the
use_grpcflag is correctly passed to the client method.src/scanoss/scanossgrpc.py (11)
76-104: LGTM! REST endpoint registry correctly defined.The
REST_ENDPOINTSmap provides a clean, maintainable registry of REST paths and HTTP methods for each RPC operation. The structure aligns with the unified transport layer introduced in this PR.
251-268: LGTM! Echo methods correctly refactored to use unified dispatcher.Both
deps_echoandcrypto_echonow route through_call_api, enabling REST/gRPC selection based on configuration.
269-336: LGTM! Dependency processing correctly refactored for unified transport.The
use_grpcflag is correctly propagated through the parallel processing pipeline, and_process_dep_filenow routes through_call_api, enabling per-call transport selection.
338-376: LGTM! Vulnerabilities and semgrep methods correctly refactored.Both
get_vulnerabilities_jsonandget_semgrep_jsonnow accept ause_grpcparameter and route through_call_api, enabling REST/gRPC selection.
378-412: LGTM! Component methods correctly refactored and past issue resolved.Both
search_components_jsonandget_component_versions_jsonnow accept ause_grpcparameter and propagate it to_call_api. Line 411 now correctly passesuse_grpc, addressing the previous review concern about the missing transport override.
431-467: LGTM! Unified dispatcher correctly implemented.The
_call_apimethod provides a clean abstraction for transport selection, correctly routing to_call_rpcor_call_restbased on theuse_grpcflag. The logic handles both dict and proto object inputs appropriately.
708-752: LGTM! REST GET method correctly implemented.The
_rest_getmethod provides robust GET request handling with appropriate retry logic, error handling, and request ID propagation.
754-798: LGTM! REST POST method correctly implemented.The
_rest_postmethod provides robust POST request handling with appropriate retry logic, error handling, and request ID propagation.
547-565: LGTM! Provenance method correctly refactored.
get_provenance_jsonnow accepts ause_grpcparameter and routes through_call_api, enabling REST/gRPC selection.
567-584: LGTM! Provenance origin method correctly refactored.
get_provenance_originnow accepts ause_grpcparameter and routes through_call_api, enabling REST/gRPC selection.
586-684: LGTM! All cryptography methods correctly refactored.All five cryptography methods now accept a
use_grpcparameter and route through_call_api, enabling REST/gRPC selection for all cryptography operations.
| if not (len(parts) >= MIN_SPLIT_PARTS and parts[1]): | ||
| raise ScanossCryptographyError( | ||
| f'Invalid PURL format: "{purl}".' f'It must include a version (e.g., pkg:type/name@version)' | ||
| f'Invalid PURL format: "{purl}".It must include a version (e.g., pkg:type/name@version)' |
There was a problem hiding this comment.
Fix typo in error message.
Missing space after period: ".It must" should be ". It must".
Apply this diff:
- f'Invalid PURL format: "{purl}".It must include a version (e.g., pkg:type/name@version)'
+ f'Invalid PURL format: "{purl}". It must include a version (e.g., pkg:type/name@version)'📝 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.
| f'Invalid PURL format: "{purl}".It must include a version (e.g., pkg:type/name@version)' | |
| f'Invalid PURL format: "{purl}". It must include a version (e.g., pkg:type/name@version)' |
🤖 Prompt for AI Agents
In src/scanoss/cryptography.py around line 40, fix the typo in the error message
by inserting a space after the period so the message reads 'Invalid PURL format:
"{purl}". It must include a version (e.g., pkg:type/name@version)'; update the
string accordingly.
matiasdaloia
force-pushed
the
feature/mdaloia/SP-3346-SCANOSS-PY-Allow-REST-in-crypto-commands
branch
from
f1faf4d to
6f3180a
Compare
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
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 (2)
src/scanoss/scanossgrpc.py (2)
46-46: Remove unused import.Pipeline failure indicates
ComponentsLicenseResponseis imported but never used.Apply this diff:
-from scanoss.api.licenses.v2.scanoss_licenses_pb2 import ComponentsLicenseResponse from scanoss.api.licenses.v2.scanoss_licenses_pb2_grpc import LicenseStub
70-70: Remove unused import.Pipeline failure indicates
ComponentsVulnerabilityResponseis imported but never used.Apply this diff:
-from .api.vulnerabilities.v2.scanoss_vulnerabilities_pb2 import ( - ComponentsVulnerabilityResponse, -) from .api.vulnerabilities.v2.scanoss_vulnerabilities_pb2_grpc import VulnerabilitiesStub
♻️ Duplicate comments (3)
src/scanoss/api/scanning/v2/scanoss_scanning_pb2.py (1)
41-43: Follow up: ensure CLI/router handles /v2/scanning URLs.These bindings now expect
/v2/scanning/.... Please confirmScanossGrpc(and any REST aliases) were updated so existing/api/v2/scanning/...clients keep working or get redirected. Same concern as earlier review—just want to make sure it’s closed out.src/scanoss/cryptography.py (1)
73-73: Fix typo in error message.Line 73 still has the typo flagged in a previous review: ".It must" should be ". It must" (missing space after period).
Apply this diff:
- f'Invalid PURL format: "{purl}".It must include a version (e.g., pkg:type/name@version)' + f'Invalid PURL format: "{purl}". It must include a version (e.g., pkg:type/name@version)'src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (1)
31-35: Move custom docstrings out of generated gRPC file.This file contains a "DO NOT EDIT!" header but includes custom docstrings that will be lost on the next protoc run. These should be moved upstream to the .proto service definition or to a wrapper subclass.
As noted in a previous review, revert the custom documentation in this generated file and either:
- Add the comments to the .proto service/method definitions so protoc emits them, or
- Create a wrapper module that subclasses CryptographyServicer with your documentation.
Also applies to: 127-131, 425-429
🧹 Nitpick comments (4)
src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2.py (1)
55-63: REST mapping: consider POST body for singular endpoints.Singular endpoints (GetCountryContributorsByComponent, GetOriginByComponent) are mapped without body, implying GET with query params. ComponentRequest can grow and exceed URL limits; align with plural endpoints by using POST body:"*".
src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2.py (3)
47-51: REST mapping parity for singular vs plural.Plural issues endpoint uses POST with body ""; singular uses no body (likely GET). To avoid long URLs and allow richer payloads, consider POST body:"" for the singular endpoint too.
52-55: Optional proto modeling: make severity an enum.Issue.severity is a string; consider an enum to prevent typos and enable stricter clients. Requires proto change, not a direct edit here.
30-37: Constrainprotobufpin to<7
setup.cfg and requirements.txt already includeprotobuf>=6.3.1, but should also include<7to avoid future major‐version breakage. grpcio>=1.73.1 is correctly pinned.
Diff (in both files):- protobuf>=6.3.1 + protobuf>=6.3.1,<7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/scanoss/api/common/v2/scanoss_common_pb2.py(3 hunks)src/scanoss/api/components/v2/scanoss_components_pb2.py(1 hunks)src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py(3 hunks)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2.py(1 hunks)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py(9 hunks)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2.py(1 hunks)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py(1 hunks)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2.py(1 hunks)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py(5 hunks)src/scanoss/api/licenses/v2/scanoss_licenses_pb2.py(3 hunks)src/scanoss/api/scanning/v2/scanoss_scanning_pb2.py(2 hunks)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2.py(1 hunks)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py(5 hunks)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2.py(3 hunks)src/scanoss/cli.py(5 hunks)src/scanoss/components.py(6 hunks)src/scanoss/cryptography.py(7 hunks)src/scanoss/scanossgrpc.py(9 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/scanoss/api/common/v2/scanoss_common_pb2.py
- src/scanoss/api/components/v2/scanoss_components_pb2.py
- src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py
- src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py
🧰 Additional context used
🧬 Code graph analysis (4)
src/scanoss/components.py (1)
src/scanoss/scanossgrpc.py (6)
get_vulnerabilities_json(345-363)get_semgrep_json(365-383)search_components_json(385-401)get_component_versions_json(403-419)get_provenance_origin(574-591)get_provenance_json(554-572)
src/scanoss/scanossgrpc.py (8)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (6)
Echo(67-72)Echo(132-156)SearchComponents(74-79)SearchComponents(159-183)GetComponentVersions(81-86)GetComponentVersions(186-210)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (12)
Echo(133-139)Echo(432-456)GetComponentsAlgorithms(167-178)GetComponentsAlgorithms(513-537)GetComponentsAlgorithmsInRange(206-217)GetComponentsAlgorithmsInRange(594-618)GetComponentsEncryptionHints(319-330)GetComponentsEncryptionHints(837-861)GetComponentsHintsInRange(282-291)GetComponentsHintsInRange(756-780)GetComponentsVersionsInRange(245-256)GetComponentsVersionsInRange(675-699)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (4)
Echo(62-67)Echo(116-140)GetDependencies(69-75)GetDependencies(143-167)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (6)
Echo(82-87)Echo(195-219)GetCountryContributorsByComponents(98-105)GetCountryContributorsByComponents(249-273)GetOriginByComponents(125-132)GetOriginByComponents(330-354)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (4)
Echo(67-72)Echo(138-162)GetComponentsIssues(83-90)GetComponentsIssues(192-216)src/scanoss/api/scanning/v2/scanoss_scanning_pb2_grpc.py (2)
Echo(57-62)Echo(98-122)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (4)
Echo(82-88)Echo(220-244)GetComponentsVulnerabilities(154-166)GetComponentsVulnerabilities(382-406)src/scanoss/scanossbase.py (3)
print_stderr(45-49)print_trace(65-70)print_debug(58-63)
src/scanoss/cryptography.py (2)
src/scanoss/cli.py (1)
print_stderr(88-92)src/scanoss/scanossgrpc.py (5)
get_crypto_algorithms_in_range_for_purl(613-631)get_crypto_algorithms_for_purl(593-611)get_encryption_hints_in_range_for_purl(653-671)get_encryption_hints_for_purl(633-651)get_versions_in_range_for_purl(673-691)
src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (3)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (2)
Echo(67-72)Echo(132-156)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (2)
Echo(82-87)Echo(195-219)src/scanoss/api/licenses/v2/scanoss_licenses_pb2_grpc.py (2)
Echo(72-78)Echo(170-194)
🪛 GitHub Actions: Lint
src/scanoss/scanossgrpc.py
[error] 46-46: F401: Unused import 'ComponentsLicenseResponse' from scanoss.api.licenses.v2.scanoss_licenses_pb2. Remove unused import.
[error] 70-70: F401: Unused import 'ComponentsVulnerabilityResponse' from .api.vulnerabilities.v2.scanoss_vulnerabilities_pb2. Remove unused import.
⏰ 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 (54)
src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2.py (1)
52-65: Confirm REST router uses the new /v2/vulnerabilities bindings.These serialized options drop the
/apiprefix for every vulnerabilities endpoint. UnlessScanossGrpc(and any other REST routers) now dispatch to/v2/..., existing clients hitting/api/v2/...will break. Please confirm the router and CLI layers were updated or that both paths remain available.src/scanoss/api/licenses/v2/scanoss_licenses_pb2.py (1)
45-53: Verify license REST aliases for /v2 paths.Switching these bindings to
/v2/licenses/...removes the old/api/v2/...URLs. Please double-check that the REST gateway (and CLI plumbing) now points to/v2/..., or that you keep backward-compatible aliases so current users don’t lose the endpoints.src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2.py (1)
51-61: Good: deprecation flags wired on legacy RPCs.Deprecation annotated on GetComponentContributors/GetComponentOrigin is correct; ensure CLI/routes prefer the new Component(s) variants and emit deprecation notices.
src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2.py (1)
46-48: Good: legacy GetIssues marked deprecated.Deprecation option set; ensure higher layers route to GetComponent(s)Issues.
src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (4)
50-74: New GeoProvenance RPCs wired on the client stub.Additions for components/component variants look correct and match new request/response types.
156-180: Server registration covers all new handlers.Handlers and serializers/deserializers align with proto types. Good parity with the stub.
82-97: Deprecation docstrings + UNIMPLEMENTED stubs are fine.Clear guidance on replacements and safe fallbacks via UNIMPLEMENTED.
50-74: Packaging already pins grpcio>=1.73.1 requirements.txt and setup.cfg both define grpcio>=1.73.1, satisfying the module’s version requirement.src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (1)
69-75: LGTM! Clear deprecation notice.The deprecation notice correctly informs users to migrate to
/v2/licenses/componentsfor dependency details. This maintains backward compatibility while guiding users toward the preferred endpoint.src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2.py (1)
30-73: Generated descriptor metadata updates.These changes update descriptor options and serialized boundaries from the protobuf compiler regeneration. The past review comment regarding protobuf version pinning (≥6.31.0 required but packaging pins >3.19.1) remains valid and should be addressed in packaging files.
src/scanoss/components.py (7)
59-80: LGTM! Instance-level gRPC flag added.The
use_grpcparameter (defaultFalse) is properly added to__init__and stored as an instance variable, enabling per-instance control of REST vs. gRPC routing while preserving backward compatibility.
193-217: LGTM! Propagates use_grpc to vulnerabilities API.The method now uses
load_compsfor data loading and correctly propagatesself.use_grpcto the gRPC API call at line 210, enabling REST/gRPC routing control.
219-243: LGTM! Propagates use_grpc to semgrep API.Correctly uses
load_compsand propagatesself.use_grpcto the gRPC API call at line 236.
286-286: LGTM! Propagates use_grpc to component search API.Line 286 correctly propagates
self.use_grpcto enable routing control for component search.
322-322: LGTM! Propagates use_grpc to component versions API.Correctly propagates routing control to the component versions API call.
331-365: LGTM! Propagates use_grpc to provenance APIs.Uses
load_compsfor data loading and correctly propagatesself.use_grpcto both provenance API calls (origin at line 355 and declared at line 358).
367-397: Remove invalid use_grpc propagation suggestion
Thegrpc_api.get_licensesmethod doesn’t accept ause_grpcargument because it readsself.use_grpcinternally, so no explicit flag propagation is needed.Likely an incorrect or invalid review comment.
src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2.py (1)
30-154: LGTM! Generated cryptography protobuf updates.These changes update descriptor options, method bindings, and serialized boundaries from protobuf compiler regeneration. All changes are mechanical and expected for generated code.
src/scanoss/cli.py (8)
1060-1073: LGTM! Global --grpc flag added consistently.The global
--grpcflag is added to all relevant subparsers (vulns, scan, container-scan, crypto commands, semgrep, provenance, search, versions), providing a consistent interface for enabling gRPC support across the CLI.
2127-2161: LGTM! comp_vulns propagates use_grpc correctly.The
comp_vulnsfunction correctly passesuse_grpc=args.grpcto theComponentsconstructor at line 2158, enabling REST/gRPC routing control from the CLI flag.
2164-2197: LGTM! comp_semgrep propagates use_grpc correctly.Correctly propagates
use_grpc=args.grpcat line 2194.
2200-2245: LGTM! comp_search propagates use_grpc correctly.Correctly propagates
use_grpc=args.grpcat line 2233.
2248-2282: LGTM! comp_versions propagates use_grpc correctly.Correctly propagates
use_grpc=args.grpcat line 2279.
2285-2318: LGTM! comp_provenance propagates use_grpc correctly.Correctly propagates
use_grpc=args.grpcat line 2315.
2321-2354: LGTM! comp_licenses propagates use_grpc correctly.Correctly propagates
use_grpc=args.grpcat line 2351, completing the consistent pattern across all component commands.
1387-1424: LGTM! Scanner propagates use_grpc correctly.The
scanfunction passesuse_grpc=args.grpcto theScannerconstructor at line 1423, enabling REST/gRPC routing control for scan operations.src/scanoss/cryptography.py (6)
22-29: Field ordering in CryptographyConfig looks good.The dataclass fields are properly ordered with required fields (purl) before optional fields with defaults.
150-151: Empty components validation correctly implemented.The validation now checks both the existence of
components_requestand whether the 'components' list is non-empty, addressing the concern from the previous review.
95-102: Config construction aligns with new fields.The factory function correctly maps all CLI arguments including the new
headeranduse_grpcfields to the CryptographyConfig.
139-139: Correctly renamed to components_request.The refactoring from
purls_requesttocomponents_requestis consistent with the broader shift to a components-based API.
154-160: use_grpc flag properly threaded through.All cryptography method calls now correctly pass
self.config.use_grpcto enable per-call transport selection.
209-226: Components request structure correctly implemented.The
_build_components_requestmethod now returns a dict with a 'components' key, where each entry contains 'purl' and 'requirement' fields. This aligns with the updated API surface in the gRPC client.src/scanoss/scanossgrpc.py (18)
84-109: REST endpoint registry is well-structured.The REST_ENDPOINTS dict provides a clear mapping from service.method keys to path/method configs, enabling centralized routing for all decoration services.
438-474: Unified _call_api routing enables clean transport selection.The method correctly routes to either gRPC or REST based on the
use_grpcflag, with proper fallback to the instance default. The dict-to-proto conversion for REST is handled cleanly.
746-790: _rest_get method is robust.The implementation includes proper error handling, retries, and timeout logic consistent with the existing _rest_post method.
792-836: _rest_post updated with request_id header.The method now accepts and sets the
x-request-idheader for tracing, aligning with the gRPC request ID flow.
302-309: Concurrent processing correctly threads use_grpc.The thread pool executor now passes
self.use_grpcto each_process_dep_filecall, enabling per-call transport selection for parallel dependency processing.
322-343: _process_dep_file signature updated for transport selection.The method now accepts a
use_grpcparameter and routes through_call_api, enabling REST/gRPC selection at the per-file level.
345-363: get_vulnerabilities_json signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for vulnerability decoration.
365-383: get_semgrep_json signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for semgrep decoration.
385-401: search_components_json signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for component search.
403-419: get_component_versions_json signature updated.The method now accepts
use_grpcand routes through_call_api, correctly forwarding the transport override (this was flagged in a previous review as fixed).
554-572: get_provenance_json signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for provenance decoration.
574-591: get_provenance_origin signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for provenance origin queries.
593-611: get_crypto_algorithms_for_purl signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for cryptography algorithm queries.
613-631: get_crypto_algorithms_in_range_for_purl signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for in-range cryptography algorithm queries.
633-651: get_encryption_hints_for_purl signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for encryption hints queries.
653-671: get_encryption_hints_in_range_for_purl signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for in-range encryption hints queries.
673-691: get_versions_in_range_for_purl signature updated.The method now accepts
use_grpcand routes through_call_api, enabling transport selection for version-in-range queries.
265-266: Echo methods correctly route through _call_api.Both
deps_echoandcrypto_echonow use the unified_call_apimethod, enabling REST fallback for health checks.src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (4)
53-62: New RPC stub methods are correctly wired.The client stub now includes GetComponentAlgorithms, GetComponentsAlgorithms, and corresponding InRange/EncryptionHints variants, all properly registered with correct request/response types.
Also applies to: 68-77, 83-92, 98-107, 113-122
154-175: New servicer methods follow standard pattern.All new servicer methods (GetComponentAlgorithms, GetComponentsAlgorithms, etc.) correctly raise UNIMPLEMENTED by default, consistent with other gRPC servicer stubs.
Also applies to: 193-214, 232-253, 271-288, 306-327
345-354: Handler registration is complete.The
add_CryptographyServicer_to_serverfunction correctly registers all new RPC methods with proper request deserializers and response serializers.Also applies to: 360-369, 375-384, 390-399, 405-414
485-537: Experimental API methods are correctly generated.The static methods in the experimental Cryptography class for GetComponentAlgorithms, GetComponentsAlgorithms, and variants are properly wired with correct paths and serialization.
Also applies to: 566-618, 647-699, 728-780, 809-861
| DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n8scanoss/api/geoprovenance/v2/scanoss-geoprovenance.proto\x12\x1cscanoss.api.geoprovenance.v2\x1a*scanoss/api/common/v2/scanoss-common.proto\x1a\x1cgoogle/api/annotations.proto\x1a.protoc-gen-openapiv2/options/annotations.proto\"2\n\x10\x44\x65\x63laredLocation\x12\x0c\n\x04type\x18\x01 \x01(\t\x12\x10\n\x08location\x18\x02 \x01(\t\"1\n\x0f\x43uratedLocation\x12\x0f\n\x07\x63ountry\x18\x01 \x01(\t\x12\r\n\x05\x63ount\x18\x02 \x01(\x05\"\xed\x02\n\x13\x43ontributorResponse\x12\x46\n\x05purls\x18\x01 \x03(\x0b\x32\x37.scanoss.api.geoprovenance.v2.ContributorResponse.Purls\x12\x35\n\x06status\x18\x02 \x01(\x0b\x32%.scanoss.api.common.v2.StatusResponse\x1a\xd2\x01\n\x05Purls\x12\x0c\n\x04purl\x18\x01 \x01(\t\x12^\n\x12\x64\x65\x63lared_locations\x18\x02 \x03(\x0b\x32..scanoss.api.geoprovenance.v2.DeclaredLocationR\x12\x64\x65\x63lared_locations\x12[\n\x11\x63urated_locations\x18\x03 \x03(\x0b\x32-.scanoss.api.geoprovenance.v2.CuratedLocationR\x11\x63urated_locations:\x02\x18\x01\"\xe2\x01\n\x15\x43omponentLocationInfo\x12\x0c\n\x04purl\x18\x01 \x01(\t\x12^\n\x12\x64\x65\x63lared_locations\x18\x02 \x03(\x0b\x32..scanoss.api.geoprovenance.v2.DeclaredLocationR\x12\x64\x65\x63lared_locations\x12[\n\x11\x63urated_locations\x18\x03 \x03(\x0b\x32-.scanoss.api.geoprovenance.v2.CuratedLocationR\x11\x63urated_locations\"\xd5\x04\n\x1d\x43omponentsContributorResponse\x12g\n\x14\x63omponents_locations\x18\x01 \x03(\x0b\x32\x33.scanoss.api.geoprovenance.v2.ComponentLocationInfoR\x14\x63omponents_locations\x12\x35\n\x06status\x18\x02 \x01(\x0b\x32%.scanoss.api.common.v2.StatusResponse:\x93\x03\x92\x41\x8f\x03\n\x8c\x03J\x89\x03{\"components_locations\":[{\"purl\":\"pkg:github/scanoss/[email protected]\",\"declared_locations\":[{\"type\":\"owner\",\"location\":\"Barcelona, Spain\"},{\"type\":\"contributor\",\"location\":\"Berlin, Germany\"}],\"curated_locations\":[{\"country\":\"Spain\",\"count\":8},{\"country\":\"Germany\",\"count\":3},{\"country\":\"United States\",\"count\":2}]}],\"status\":{\"status\":\"SUCCESS\",\"message\":\"Geo-provenance successfully retrieved\"}}\"\xce\x04\n\x1c\x43omponentContributorResponse\x12\x65\n\x13\x63omponent_locations\x18\x01 \x01(\x0b\x32\x33.scanoss.api.geoprovenance.v2.ComponentLocationInfoR\x13\x63omponent_locations\x12\x35\n\x06status\x18\x02 \x01(\x0b\x32%.scanoss.api.common.v2.StatusResponse:\x8f\x03\x92\x41\x8b\x03\n\x88\x03J\x85\x03{\"component_location\":{\"purl\":\"pkg:github/scanoss/[email protected]\",\"declared_locations\":[{\"type\":\"owner\",\"location\":\"Barcelona, Spain\"},{\"type\":\"contributor\",\"location\":\"Berlin, Germany\"}],\"curated_locations\":[{\"country\":\"Spain\",\"count\":8},{\"country\":\"Germany\",\"count\":3},{\"country\":\"United States\",\"count\":2}]},\"status\":{\"status\":\"SUCCESS\",\"message\":\"Geo-provenance successfully retrieved\"}}\",\n\x08Location\x12\x0c\n\x04name\x18\x01 \x01(\t\x12\x12\n\npercentage\x18\x02 \x01(\x02\"\\\n\x11\x43omponentLocation\x12\x0c\n\x04purl\x18\x01 \x01(\t\x12\x39\n\tlocations\x18\x02 \x03(\x0b\x32&.scanoss.api.geoprovenance.v2.Location\"\xe0\x01\n\x0eOriginResponse\x12\x41\n\x05purls\x18\x01 \x03(\x0b\x32\x32.scanoss.api.geoprovenance.v2.OriginResponse.Purls\x12\x35\n\x06status\x18\x02 \x01(\x0b\x32%.scanoss.api.common.v2.StatusResponse\x1aP\n\x05Purls\x12\x0c\n\x04purl\x18\x01 \x01(\t\x12\x39\n\tlocations\x18\x02 \x03(\x0b\x32&.scanoss.api.geoprovenance.v2.Location:\x02\x18\x01\"\xcd\x03\n\x18\x43omponentsOriginResponse\x12\x63\n\x14\x63omponents_locations\x18\x01 \x03(\x0b\x32/.scanoss.api.geoprovenance.v2.ComponentLocationR\x14\x63omponents_locations\x12\x35\n\x06status\x18\x02 \x01(\x0b\x32%.scanoss.api.common.v2.StatusResponse:\x94\x02\x92\x41\x90\x02\n\x8d\x02J\x8a\x02{\"components_locations\":[{\"purl\":\"pkg:github/scanoss/[email protected]\",\"locations\":[{\"name\":\"ES\",\"percentage\":65.5},{\"name\":\"DE\",\"percentage\":20.3},{\"name\":\"US\",\"percentage\":14.2}]}],\"status\":{\"status\":\"SUCCESS\",\"message\":\"Geo-provenance origin successfully retrieved\"}}\"\xc8\x03\n\x17\x43omponentOriginResponse\x12\x61\n\x13\x63omponent_locations\x18\x01 \x01(\x0b\x32/.scanoss.api.geoprovenance.v2.ComponentLocationR\x13\x63omponent_locations\x12\x35\n\x06status\x18\x02 \x01(\x0b\x32%.scanoss.api.common.v2.StatusResponse:\x92\x02\x92\x41\x8e\x02\n\x8b\x02J\x88\x02{\"component_locations\": {\"purl\":\"pkg:github/scanoss/[email protected]\",\"locations\":[{\"name\":\"ES\",\"percentage\":65.5},{\"name\":\"DE\",\"percentage\":20.3},{\"name\":\"US\",\"percentage\":14.2}]},\"status\":{\"status\":\"SUCCESS\",\"message\":\"Geo-provenance origin successfully retrieved\"}}2\xff\x08\n\rGeoProvenance\x12r\n\x04\x45\x63ho\x12\".scanoss.api.common.v2.EchoRequest\x1a#.scanoss.api.common.v2.EchoResponse\"!\x82\xd3\xe4\x93\x02\x1b\"\x16/v2/geoprovenance/echo:\x01*\x12\x9c\x01\n\x18GetComponentContributors\x12\".scanoss.api.common.v2.PurlRequest\x1a\x31.scanoss.api.geoprovenance.v2.ContributorResponse\")\x88\x02\x01\x82\xd3\xe4\x93\x02 \"\x1b/v2/geoprovenance/countries:\x01*\x12\xbe\x01\n\"GetCountryContributorsByComponents\x12(.scanoss.api.common.v2.ComponentsRequest\x1a;.scanoss.api.geoprovenance.v2.ComponentsContributorResponse\"1\x82\xd3\xe4\x93\x02+\"&/v2/geoprovenance/countries/components:\x01*\x12\xb7\x01\n!GetCountryContributorsByComponent\x12\'.scanoss.api.common.v2.ComponentRequest\x1a:.scanoss.api.geoprovenance.v2.ComponentContributorResponse\"-\x82\xd3\xe4\x93\x02\'\x12%/v2/geoprovenance/countries/component\x12\x8e\x01\n\x12GetComponentOrigin\x12\".scanoss.api.common.v2.PurlRequest\x1a,.scanoss.api.geoprovenance.v2.OriginResponse\"&\x88\x02\x01\x82\xd3\xe4\x93\x02\x1d\"\x18/v2/geoprovenance/origin:\x01*\x12\xa9\x01\n\x15GetOriginByComponents\x12(.scanoss.api.common.v2.ComponentsRequest\x1a\x36.scanoss.api.geoprovenance.v2.ComponentsOriginResponse\".\x82\xd3\xe4\x93\x02(\"#/v2/geoprovenance/origin/components:\x01*\x12\xa2\x01\n\x14GetOriginByComponent\x12\'.scanoss.api.common.v2.ComponentRequest\x1a\x35.scanoss.api.geoprovenance.v2.ComponentOriginResponse\"*\x82\xd3\xe4\x93\x02$\x12\"/v2/geoprovenance/origin/componentB\xa4\x02Z;github.com/scanoss/papi/api/geoprovenancev2;geoprovenancev2\x92\x41\xe3\x01\x12}\n\x1eSCANOSS GEO Provenance Service\"V\n\x15scanoss-geoprovenance\x12(https://github.com/scanoss/geoprovenance\x1a\x13[email protected]\x03\x32.0*\x01\x01\x32\x10\x61pplication/json:\x10\x61pplication/jsonR;\n\x03\x34\x30\x34\x12\x34\n*Returned when the resource does not exist.\x12\x06\n\x04\x9a\x02\x01\x07\x62\x06proto3') | ||
|
|
||
| _globals = globals() | ||
| _builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals) | ||
| _builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, 'scanoss.api.geoprovenance.v2.scanoss_geoprovenance_pb2', _globals) | ||
| if not _descriptor._USE_C_DESCRIPTORS: | ||
| _globals['DESCRIPTOR']._loaded_options = None | ||
| _globals['DESCRIPTOR']._serialized_options = b'Z;github.com/scanoss/papi/api/geoprovenancev2;geoprovenancev2\222A\343\001\022}\n\036SCANOSS GEO Provenance Service\"V\n\025scanoss-geoprovenance\022(https://github.com/scanoss/geoprovenance\032\023[email protected]\0032.0*\001\0012\020application/json:\020application/jsonR;\n\003404\0224\n*Returned when the resource does not exist.\022\006\n\004\232\002\001\007' |
There was a problem hiding this comment.
🧩 Analysis chain
Pin protobuf/grpc versions to match generated code.
This file enforces protobuf runtime 6.31.0 via ValidateProtobufRuntimeVersion. Ensure packaging pins protobuf>=6.31.0 (<7) and grpcio>=1.73.1 project‑wide to avoid RuntimeError at import.
Run to locate/inspect pins:
If missing, add constraints (example for requirements.txt):
+protobuf>=6.31.0,<7
+grpcio>=1.73.1,<2🏁 Script executed:
#!/bin/bash
# Find packaging manifests and print protobuf/grpcio constraints
fd -a 'pyproject.toml' 'setup.cfg' 'setup.py' 'requirements*.txt'
rg -nP '^\s*(protobuf|grpcio)\b.*' -C2 || trueLength of output: 926
Update protobuf version constraint to ≥6.31.0,<7
In setup.cfg (line 33) and requirements.txt (line 6), change
protobuf>=6.3.1
to
protobuf>=6.31.0,<7
to satisfy ValidateProtobufRuntimeVersion and prevent import-time RuntimeError.
🤖 Prompt for AI Agents
In src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2.py around lines 30
to 37, the project requires updating the protobuf runtime version constraint to
satisfy ValidateProtobufRuntimeVersion; change the protobuf requirement in
setup.cfg (line 33) and requirements.txt (line 6) from "protobuf>=6.3.1" to
"protobuf>=6.31.0,<7" and then reinstall/update dependencies (pip install -r
requirements.txt or pip install .) to ensure imports no longer raise the
RuntimeError.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/scanoss/scanossgrpc.py (1)
306-316: Merge status should pick the worst severity, not the last non-success.Current logic can downgrade a prior FAILED to SUCCESS_WITH_WARNINGS if it appears later.
Apply this diff:
- success_status = 'SUCCESS' - merged_response = {'files': [], 'status': {'status': success_status, 'message': 'Success'}} + success_status = 'SUCCESS' + status_order = {'FAILED': 3, 'FAILED_WITH_WARNINGS': 2, 'SUCCESS_WITH_WARNINGS': 1, 'SUCCESS': 0} + worst_status = success_status + worst_message = 'Success' + merged_response = {'files': [], 'status': {'status': success_status, 'message': worst_message}} @@ - # Overwrite the status if any of the responses was not successful - if 'status' in response and response['status']['status'] != success_status: - merged_response['status'] = response['status'] + st = response.get('status', {}) + code = st.get('status') + if code and status_order.get(code, 0) > status_order[worst_status]: + worst_status = code + worst_message = st.get('message', worst_message) - return merged_response + merged_response['status'] = {'status': worst_status, 'message': worst_message} + return merged_response
♻️ Duplicate comments (1)
src/scanoss/scanossgrpc.py (1)
834-872: Don’t stripstatus; don’t null-out REST failures (breaks aggregation).Removing
response['status']and returningNoneon REST failures hides per-file outcomes in callers likeget_dependencies_json(). Keep status and always return the payload; let callers decide severity.Apply this diff:
- # Check for status in response and validate - if response and 'status' in response: - status_dict = response.get('status', {}) - if status_dict.get('status') != 'SUCCESS': - self.print_stderr(f'Request failed (rqId: {request_id}): {status_dict.get("message", "Unknown error")}') - if status_dict.get('status') in ['FAILED', 'FAILED_WITH_WARNINGS']: - return None - del response['status'] + # Surface status to callers for consistent aggregation + if response and 'status' in response: + status_dict = response.get('status', {}) + if status_dict.get('status') != 'SUCCESS': + self.print_stderr( + f'Request {status_dict.get("status")} (rqId: {request_id}): ' + f'{status_dict.get("message", "Unknown error")}' + ) return response
🧹 Nitpick comments (4)
src/scanoss/scanossgrpc.py (4)
417-433: Enable REST for folder hash scan via_call_api(endpoint already mapped).You have
scanning.FolderHashScanin REST_ENDPOINTS butfolder_hash_scan()always uses gRPC. Route through_call_apito honoruse_grpc.Apply this diff:
- return self._call_rpc( - self.scanning_stub.FolderHashScan, - request, - HFHRequest, - 'Sending folder hash scan data (rqId: {rqId})...', - ) + return self._call_api( + 'scanning.FolderHashScan', + self.scanning_stub.FolderHashScan, + request, + HFHRequest, + 'Sending folder hash scan data (rqId: {rqId})...', + )Also applies to: 104-105
361-371: Docstring mentions deprecated RPC name.Update to “Semgrep GetComponentsIssues”.
- Client function to call the rpc for Semgrep GetIssues + Client function to call the rpc for Semgrep GetComponentsIssues
183-186: Duplicate User-Agent headers.Keep a single canonical
User-Agentheader.- self.headers['User-Agent'] = user_agent - self.headers['user-agent'] = user_agent + self.headers['User-Agent'] = user_agent
195-212: Harden HTTP session for concurrency (pooling).Sessions are commonly shared but not guaranteed thread-safe; mount adapters with a larger pool to avoid contention under ThreadPoolExecutor.
+from requests.adapters import HTTPAdapter @@ else: self.session = requests.sessions.Session() + # Increase pool for parallel REST calls + adapter = HTTPAdapter(pool_connections=MAX_CONCURRENT_REQUESTS * 2, pool_maxsize=MAX_CONCURRENT_REQUESTS * 2) + self.session.mount('https://', adapter) + self.session.mount('http://', adapter)Also applies to: 297-305
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(2 hunks)src/scanoss/__init__.py(1 hunks)src/scanoss/scanossgrpc.py(9 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/scanoss/scanossgrpc.py (6)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (4)
SearchComponents(74-79)SearchComponents(159-183)GetComponentVersions(81-86)GetComponentVersions(186-210)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (2)
GetDependencies(69-75)GetDependencies(143-167)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (4)
GetCountryContributorsByComponents(98-105)GetCountryContributorsByComponents(249-273)GetOriginByComponents(125-132)GetOriginByComponents(330-354)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (10)
GetComponentsAlgorithms(167-178)GetComponentsAlgorithms(513-537)GetComponentsAlgorithmsInRange(206-217)GetComponentsAlgorithmsInRange(594-618)GetComponentsEncryptionHints(319-330)GetComponentsEncryptionHints(837-861)GetComponentsHintsInRange(282-291)GetComponentsHintsInRange(756-780)GetComponentsVersionsInRange(245-256)GetComponentsVersionsInRange(675-699)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (2)
GetComponentsIssues(83-90)GetComponentsIssues(192-216)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (2)
GetComponentsVulnerabilities(154-166)GetComponentsVulnerabilities(382-406)
🔇 Additional comments (3)
CHANGELOG.md (1)
12-15: Changelog entry and link look good.Release date and compare link align with the version bump.
Also applies to: 675-676
src/scanoss/__init__.py (1)
25-25: Version bump is consistent with CHANGELOG.No further action.
src/scanoss/scanossgrpc.py (1)
79-106: Verify Dependencies REST endpoint before mapping
The proto doesn’t define a REST path forGetDependencies;/v2/licenses/componentsapplies to licenses, not dependencies. Inspect the generated REST-gateway registration (e.g. server.go or gateway code) to determine the correct HTTP endpoint.Likely an incorrect or invalid review comment.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/scanoss/scanossgrpc.py (1)
827-871: Fix exception handler order (Critical).Same issue as
_rest_get:requests.exceptions.RequestException(line 852) catches specific exceptions before their handlers can execute.Reorder the except clauses:
try: response = self.session.post(uri, headers=headers, json=data, timeout=self.timeout) response.raise_for_status() # Raises an HTTPError for bad responses return response.json() - except requests.exceptions.RequestException as e: - self.print_stderr(f'Error: Problem posting data to {uri}: {e}') - except (requests.exceptions.SSLError, requests.exceptions.ProxyError) as e: + except (requests.exceptions.SSLError, requests.exceptions.ProxyError) as e: self.print_stderr(f'ERROR: Exception ({e.__class__.__name__}) POSTing data - {e}.') raise Exception(f'ERROR: The SCANOSS Decoration API request failed for {uri}') from e except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as e: if retry > self.retry_limit: # Timed out retry_limit or more times, fail self.print_stderr(f'ERROR: {e.__class__.__name__} POSTing decoration data ({request_id}): {e}') raise Exception( f'ERROR: The SCANOSS Decoration API request timed out ({e.__class__.__name__}) for {uri}' ) from e else: self.print_stderr(f'Warning: {e.__class__.__name__} communicating with {self.url}. Retrying...') time.sleep(5) + except requests.exceptions.RequestException as e: + self.print_stderr(f'Error: Problem posting data to {uri}: {e}') except Exception as e: self.print_stderr( f'ERROR: Exception ({e.__class__.__name__}) POSTing data ({request_id}) to {uri}: {e}' ) raise Exception(f'ERROR: The SCANOSS Decoration API request failed for {uri}') from e
♻️ Duplicate comments (1)
src/scanoss/scanossgrpc.py (1)
781-825: Fix exception handler order (Critical).
requests.exceptions.RequestExceptionis a superclass that catchesSSLError,ProxyError,Timeout, andConnectionError. Catching it first (line 806) prevents the more specific handlers below from executing, breaking retry logic and error reporting.Reorder the except clauses:
try: response = self.session.get(uri, headers=headers, params=params, timeout=self.timeout) response.raise_for_status() # Raises an HTTPError for bad responses return response.json() - except requests.exceptions.RequestException as e: - self.print_stderr(f'Error: Problem sending GET request to {uri}: {e}') - except (requests.exceptions.SSLError, requests.exceptions.ProxyError) as e: + except (requests.exceptions.SSLError, requests.exceptions.ProxyError) as e: self.print_stderr(f'ERROR: Exception ({e.__class__.__name__}) sending GET request - {e}.') raise Exception(f'ERROR: The SCANOSS API GET request failed for {uri}') from e except (requests.exceptions.Timeout, requests.exceptions.ConnectionError) as e: if retry > self.retry_limit: # Timed out retry_limit or more times, fail self.print_stderr(f'ERROR: {e.__class__.__name__} sending GET request ({request_id}): {e}') raise Exception( f'ERROR: The SCANOSS API GET request timed out ({e.__class__.__name__}) for {uri}' ) from e else: self.print_stderr(f'Warning: {e.__class__.__name__} communicating with {self.url}. Retrying...') time.sleep(5) + except requests.exceptions.RequestException as e: + self.print_stderr(f'Error: Problem sending GET request to {uri}: {e}') except Exception as e: self.print_stderr( f'ERROR: Exception ({e.__class__.__name__}) sending GET request ({request_id}) to {uri}: {e}' ) raise Exception(f'ERROR: The SCANOSS API GET request failed for {uri}') from e
🧹 Nitpick comments (1)
src/scanoss/scanossgrpc.py (1)
537-577: Consider simplifying return logic (optional).The method correctly handles all REST status codes but triggers a linting warning for too many return statements (7 > 6). While the logic is sound, consolidating the status checks could improve maintainability.
Consider this refactor:
- # Handle case where status might be a string directly - if isinstance(status, str): - status_str = status.upper() - if status_str == ScanossRESTStatusCode.SUCCESS.value: - return True - elif status_str == ScanossRESTStatusCode.SUCCEEDED_WITH_WARNINGS.value: - self.print_stderr(f'Succeeded with warnings (rqId: {request_id}): {message}') - return True - elif status_str == ScanossRESTStatusCode.WARNING.value: - self.print_stderr(f'Failed with warnings (rqId: {request_id}): {message}') - return False - elif status_str == ScanossRESTStatusCode.FAILED.value: - self.print_stderr(f'Unsuccessful (rqId: {request_id}): {message}') - return False - else: - self.print_debug(f'Unknown status "{status_str}" (rqId: {request_id}). Assuming success.') - return True + # Handle case where status might be a string directly + if isinstance(status, str): + status_str = status.upper() + success_statuses = {ScanossRESTStatusCode.SUCCESS.value, ScanossRESTStatusCode.SUCCEEDED_WITH_WARNINGS.value} + if status_str in success_statuses: + if status_str == ScanossRESTStatusCode.SUCCEEDED_WITH_WARNINGS.value: + self.print_stderr(f'Succeeded with warnings (rqId: {request_id}): {message}') + return True + elif status_str == ScanossRESTStatusCode.WARNING.value: + self.print_stderr(f'Failed with warnings (rqId: {request_id}): {message}') + return False + elif status_str == ScanossRESTStatusCode.FAILED.value: + self.print_stderr(f'Unsuccessful (rqId: {request_id}): {message}') + return False + # Unknown status + self.print_debug(f'Unknown status "{status_str}" (rqId: {request_id}). Assuming success.') + return True
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/scanoss/cli.py(5 hunks)src/scanoss/components.py(7 hunks)src/scanoss/scanossgrpc.py(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/scanoss/scanossgrpc.py (8)
src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (4)
SearchComponents(74-79)SearchComponents(159-183)GetComponentVersions(81-86)GetComponentVersions(186-210)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (2)
GetDependencies(69-75)GetDependencies(143-167)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (2)
GetComponentsAlgorithms(167-178)GetComponentsAlgorithms(513-537)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (4)
GetCountryContributorsByComponents(98-105)GetCountryContributorsByComponents(249-273)GetOriginByComponents(125-132)GetOriginByComponents(330-354)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (2)
GetComponentsIssues(83-90)GetComponentsIssues(192-216)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (2)
GetComponentsVulnerabilities(154-166)GetComponentsVulnerabilities(382-406)src/scanoss/scanossbase.py (3)
print_stderr(45-49)print_debug(58-63)print_trace(65-70)src/scanoss/components.py (1)
get_licenses(367-397)
src/scanoss/components.py (1)
src/scanoss/scanossgrpc.py (7)
get_vulnerabilities_json(350-368)get_semgrep_json(370-388)search_components_json(390-406)get_component_versions_json(408-424)get_provenance_origin(621-638)get_provenance_json(601-619)get_licenses(740-757)
🪛 GitHub Actions: Lint
src/scanoss/scanossgrpc.py
[error] 537-537: Ruff: PLR0911 Too many return statements (7 > 6).
⏰ 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 (36)
src/scanoss/components.py (10)
80-80: LGTM: Instance flag correctly stored.The
use_grpcflag is properly saved as an instance variable for propagation to downstream API calls.
210-210: LGTM: Correctly threads gRPC flag.The
use_grpcflag is properly forwarded to the gRPC API call, enabling per-instance REST/gRPC routing.
229-229: LGTM: Aligned with component-shaped request.Switching from
load_purlstoload_compsis consistent with the API's expectation of component-shaped requests.
236-236: LGTM: gRPC flag correctly forwarded.The
use_grpcflag is properly threaded through to the API client.
286-286: LGTM: Flag correctly propagated.
322-322: LGTM: Flag forwarded correctly.
347-347: LGTM: Component-shaped request aligned.
355-355: LGTM: Flag correctly threaded.
358-358: LGTM: Flag correctly threaded.
390-390: LGTM: Flag forwarded correctly.src/scanoss/cli.py (6)
1059-1074: LGTM: Centralized gRPC flag wiring.The global
--grpcflag is consistently added across all relevant parsers, enabling unified REST/gRPC routing at the CLI level.
2158-2158: LGTM: CLI flag correctly threaded.
2194-2194: LGTM: Flag correctly passed.
2233-2233: LGTM: Flag correctly passed.
2279-2279: LGTM: Flag correctly passed.
2315-2315: LGTM: Flag correctly passed.src/scanoss/scanossgrpc.py (20)
79-106: LGTM: REST endpoint registry is well-structured.The centralized
REST_ENDPOINTSmapping provides a clean separation between logical RPC names and REST paths/methods, enabling unified routing.
125-131: LGTM: REST status code enum mirrors gRPC.The
ScanossRESTStatusCodeenum provides parallel status handling for REST responses, maintaining consistency with gRPC status codes.
443-479: LGTM: Unified API routing is clean.The
_call_apimethod provides a clean abstraction for REST/gRPC routing, properly handling per-call overrides and request type conversions.
873-905: LGTM: REST call wrapper is well-structured.The
_call_restmethod provides clean endpoint lookup, method routing, and status checking for REST responses.
270-270: LGTM: Migrated to unified API.
279-279: LGTM: Migrated to unified API.
281-281: LGTM: Signature updated correctly.
307-348: LGTM: Concurrent processing correctly threads gRPC flag.The
_process_dep_filemethod properly receives and forwards theuse_grpcflag through concurrent processing, enabling per-file REST/gRPC routing.
350-368: LGTM: Migrated to unified API with gRPC override.
370-388: LGTM: Migrated to unified API with gRPC override.
390-406: LGTM: Migrated to unified API with gRPC override.
408-424: LGTM: Migrated to unified API with gRPC override.
601-619: LGTM: Migrated to unified API with gRPC override.
621-638: LGTM: Migrated to unified API with gRPC override.
640-658: LGTM: Migrated to unified API with gRPC override.
660-678: LGTM: Migrated to unified API with gRPC override.
680-698: LGTM: Migrated to unified API with gRPC override.
700-718: LGTM: Migrated to unified API with gRPC override.
720-738: LGTM: Migrated to unified API with gRPC override.
740-757: LGTM: Migrated to unified API with gRPC override.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/scanoss/scanossgrpc.py (1)
541-583: Clarify fallback logic for non-string status values.The method assumes success when
statusis not a string (line 582), but this fallback is reached even ifstatusexists but is an unexpected type (e.g., int, dict). Consider handling these cases more explicitly or logging a warning instead of silently assuming success.Also, there's a typo in the comment at line 582: "asume" should be "assume".
Apply this diff to fix the typo and improve the fallback logic:
- # Otherwise asume success - self.print_debug(f'Unexpected status type {type(status)} (rqId: {request_id}). Assuming success.') + # Otherwise assume success + if status is not None: + self.print_stderr(f'Warning: Unexpected status type {type(status)} (rqId: {request_id}). Assuming success.') return ret_val
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/scanoss/scanossgrpc.py(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/scanoss/scanossgrpc.py (10)
src/scanoss/scanossbase.py (4)
ScanossBase(28-107)print_stderr(45-49)print_debug(58-63)print_trace(65-70)src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (6)
Echo(67-72)Echo(132-156)SearchComponents(74-79)SearchComponents(159-183)GetComponentVersions(81-86)GetComponentVersions(186-210)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (4)
Echo(62-67)Echo(116-140)GetDependencies(69-75)GetDependencies(143-167)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (12)
Echo(133-139)Echo(432-456)GetComponentsAlgorithms(167-178)GetComponentsAlgorithms(513-537)GetComponentsAlgorithmsInRange(206-217)GetComponentsAlgorithmsInRange(594-618)GetComponentsEncryptionHints(319-330)GetComponentsEncryptionHints(837-861)GetComponentsHintsInRange(282-291)GetComponentsHintsInRange(756-780)GetComponentsVersionsInRange(245-256)GetComponentsVersionsInRange(675-699)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (6)
Echo(82-87)Echo(195-219)GetCountryContributorsByComponents(98-105)GetCountryContributorsByComponents(249-273)GetOriginByComponents(125-132)GetOriginByComponents(330-354)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (4)
Echo(67-72)Echo(138-162)GetComponentsIssues(83-90)GetComponentsIssues(192-216)src/scanoss/api/scanning/v2/scanoss_scanning_pb2_grpc.py (2)
Echo(57-62)Echo(98-122)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (4)
Echo(82-88)Echo(220-244)GetComponentsVulnerabilities(154-166)GetComponentsVulnerabilities(382-406)src/scanoss/components.py (1)
get_licenses(367-397)src/scanoss/scanossapi.py (1)
load_generic_headers(270-283)
⏰ 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 (7)
src/scanoss/scanossgrpc.py (7)
79-106: LGTM! Clean centralized endpoint mapping.The REST_ENDPOINTS dictionary provides a clear and maintainable mapping from logical RPC names to REST paths and HTTP methods, enabling unified routing.
127-135: LGTM! REST status codes properly defined.The ScanossRESTStatusCode enum mirrors the gRPC status codes using string values, providing type-safe status checking for REST responses.
446-482: LGTM! Unified API routing implemented correctly.The _call_api method cleanly abstracts transport selection, properly handling per-call use_grpc overrides and converting between dict and protobuf formats as needed.
787-831: LGTM! REST GET implementation with proper exception handling.The _rest_get method correctly implements retry logic and exception handling with specific exceptions caught before generic ones.
833-877: LGTM! REST POST implementation consistent with GET.The _rest_post method properly implements retry logic and exception handling, maintaining consistency with _rest_get.
879-911: LGTM! REST orchestration properly implemented.The _call_rest method correctly looks up endpoints, constructs URLs, routes to appropriate HTTP methods, and validates responses. The status field is now preserved in the response, addressing previous feedback.
266-763: Consistent refactoring across all API methods.All API methods (dependencies, cryptography, vulnerabilities, semgrep, components, geoprovenance, licenses) have been consistently updated to:
- Accept optional
use_grpcparameters- Route through the unified
_call_apimethod- Pass through the transport override
This provides a clean, maintainable architecture for supporting both REST and gRPC transports.
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/scanoss/scanossgrpc.py (1)
266-282: Consider adding per-calluse_grpcoverride for consistency.Both
deps_echo()andcrypto_echo()now route through_call_apibut don't accept ause_grpcparameter, unlike most other wrapper methods in this file. If callers might need to override the transport choice for echo calls, add the parameter for consistency.Apply this diff to add the parameter:
-def deps_echo(self, message: str = 'Hello there!') -> Optional[dict]: +def deps_echo(self, message: str = 'Hello there!', use_grpc: Optional[bool] = None) -> Optional[dict]: """ Send Echo message to the Dependency service :param self: :param message: Message to send (default: Hello there!) + :param use_grpc: Whether to use gRPC or REST (None = use instance default) :return: echo or None """ - return self._call_api('dependencies.Echo', self.dependencies_stub.Echo, {'message': message}, EchoRequest) + return self._call_api('dependencies.Echo', self.dependencies_stub.Echo, {'message': message}, EchoRequest, use_grpc=use_grpc) -def crypto_echo(self, message: str = 'Hello there!') -> Optional[dict]: +def crypto_echo(self, message: str = 'Hello there!', use_grpc: Optional[bool] = None) -> Optional[dict]: """ Send Echo message to the Cryptography service :param self: :param message: Message to send (default: Hello there!) + :param use_grpc: Whether to use gRPC or REST (None = use instance default) :return: echo or None """ - return self._call_api('cryptography.Echo', self.crypto_stub.Echo, {'message': message}, EchoRequest) + return self._call_api('cryptography.Echo', self.crypto_stub.Echo, {'message': message}, EchoRequest, use_grpc=use_grpc)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/scanoss/scanossgrpc.py(14 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/scanoss/scanossgrpc.py (9)
src/scanoss/scanossbase.py (4)
ScanossBase(28-107)print_stderr(45-49)print_debug(58-63)print_trace(65-70)src/scanoss/api/components/v2/scanoss_components_pb2_grpc.py (4)
SearchComponents(74-79)SearchComponents(159-183)GetComponentVersions(81-86)GetComponentVersions(186-210)src/scanoss/api/dependencies/v2/scanoss_dependencies_pb2_grpc.py (2)
GetDependencies(69-75)GetDependencies(143-167)src/scanoss/api/geoprovenance/v2/scanoss_geoprovenance_pb2_grpc.py (4)
GetCountryContributorsByComponents(98-105)GetCountryContributorsByComponents(249-273)GetOriginByComponents(125-132)GetOriginByComponents(330-354)src/scanoss/api/cryptography/v2/scanoss_cryptography_pb2_grpc.py (10)
GetComponentsAlgorithms(167-178)GetComponentsAlgorithms(513-537)GetComponentsAlgorithmsInRange(206-217)GetComponentsAlgorithmsInRange(594-618)GetComponentsEncryptionHints(319-330)GetComponentsEncryptionHints(837-861)GetComponentsHintsInRange(282-291)GetComponentsHintsInRange(756-780)GetComponentsVersionsInRange(245-256)GetComponentsVersionsInRange(675-699)src/scanoss/api/semgrep/v2/scanoss_semgrep_pb2_grpc.py (2)
GetComponentsIssues(83-90)GetComponentsIssues(192-216)src/scanoss/api/vulnerabilities/v2/scanoss_vulnerabilities_pb2_grpc.py (2)
GetComponentsVulnerabilities(154-166)GetComponentsVulnerabilities(382-406)src/scanoss/components.py (1)
get_licenses(367-397)src/scanoss/scanossapi.py (1)
load_generic_headers(270-283)
⏰ 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 (13)
src/scanoss/scanossgrpc.py (13)
79-106: Well-structured REST endpoint mapping.The centralized
REST_ENDPOINTSdictionary provides a clean, maintainable way to map logical RPC names to REST paths and HTTP methods. The consistent structure makes it easy to add new endpoints.
127-134: LGTM!The
ScanossRESTStatusCodeenum appropriately mirrors the gRPC status codes and uses string values that match the REST API response format.
284-351: LGTM!The refactored dependency processing correctly:
- Routes through
_call_apiwith per-calluse_grpcsupport- Processes files in parallel using ThreadPoolExecutor
- Aggregates responses and preserves failure status
- Propagates the instance
use_grpcsetting to parallel workers
353-391: LGTM!Both
get_vulnerabilities_json()andget_semgrep_json()correctly route through_call_apiwith properuse_grpcparameter support and appropriate request types.
393-427: LGTM!Both component search methods correctly route through
_call_apiwith proper request types (CompSearchRequest,CompVersionRequest) anduse_grpcparameter support.
446-482: LGTM!The unified
_call_api()dispatcher cleanly handles routing between gRPC and REST based on theuse_grpcflag, with proper fallback to the instance default and appropriate request conversion for REST.
484-515: LGTM!The
_call_rpc()method correctly handles gRPC calls with proper request conversion, metadata enrichment, status checking, and response conversion to dictionary format.
517-539: LGTM!The gRPC status checking correctly distinguishes between success states, warnings, and failures, with appropriate return values and logging.
607-744: LGTM!All provenance and cryptography methods are consistently refactored to route through
_call_apiwith properuse_grpcparameter support and appropriate request types.
746-763: LGTM!The
get_licenses()method correctly routes through_call_apiwith properuse_grpcparameter support and appropriate request type.
787-837: LGTM!The
_rest_get()method correctly handles REST GET requests with proper retry logic, timeout handling, and exception handling in the right order (specific exceptions before generic ones).
839-923: LGTM!Both
_rest_post()and_call_rest()are well-structured:
- Consistent exception handling with proper ordering
- Proper retry logic and timeout handling
- Clean routing based on HTTP method
- Integrated status checking for REST responses
541-583: Clarify fallback logic for unexpected status types.The method correctly handles all expected REST status codes, but the fallback at line 582 prints a debug message about assuming success but then falls through without an explicit return statement. While Python will return
Noneby default, this might not be the intended behavior.Apply this diff to make the success assumption explicit:
# Otherwise asume success self.print_debug(f'Unexpected status type {type(status)} (rqId: {request_id}). Assuming success.') - return ret_val + return TrueAlternatively, if you want to preserve the
ret_valvariable's last value, ensure it's initialized properly before the string check:status = status_dict.get('status') message = status_dict.get('message', '') - ret_val = True # Handle case where status might be a string directly if isinstance(status, str): status_str = status.upper() + ret_val = True if status_str == ScanossRESTStatusCode.SUCCESS.value:Likely an incorrect or invalid review comment.
Summary by CodeRabbit
New Features
Improvements
Deprecations
Chore