Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on migrating a batch of existing integrations to a new, standardized structure. The changes include updating Python versions, implementing new actions, and adding HTML widgets to improve the user experience and functionality of these integrations within the platform. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
9086554 to
4611bf4
Compare
|
❌ Marketplace Validation Failed Click to view the full reportValidation Report🧩 IntegrationsPre-Build Stagemc_afee_active_response
websense
cisco_orbital
splash
mc_afee_atd
site24x7
|
|
❌ Marketplace Validation Failed Click to view the full reportValidation Report🧩 IntegrationsPre-Build Stagesplash
websense
cisco_orbital
site24x7
mc_afee_atd
mc_afee_active_response
|
There was a problem hiding this comment.
Code Review
This pull request introduces new integrations for Cisco Orbital, CyberArk Credential Provider, Google Forms, McAfee Active Response, and McAfee ATD. The review feedback primarily addresses violations of the repository's style guide, emphasizing the need for mandatory type annotations, Google Style docstrings, and standardized Ping action messages. Additional recommendations include replacing generic exceptions with specific custom ones, fixing typos, updating legacy Python 2 code patterns, and ensuring compliance with naming conventions for resource files and Python version specifications.
I am having trouble creating individual review comments. Click here to see my feedback.
content/response_integrations/google/mc_afee_active_response/core/McAfeeActiveResponseManager.py (260-262)
Raising a generic Exception is discouraged. Please use a more specific exception class, like the custom McAfeeActiveResponseError defined in this file, to provide better error handling context. This also applies to a similar case on line 322.
raise McAfeeActiveResponseError(
"ERROR: Filter-by, filter-operator & filter-value has to be inserted or none of them."
)
content/response_integrations/google/mc_afee_active_response/actions/Ping.py (24)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_active_response/actions/Search.py (25)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/cisco_orbital/actions/Ping.py (71-75)
The success and failure messages for the Ping action are hardcoded. The style guide (lines 181-185) requires using a template for these messages. Please use the PROVIDER_NAME constant to make the messages dynamic and adhere to the standard.
output_message = f"Successfully connected to the {PROVIDER_NAME} server with the provided connection parameters!"
except Exception as e:
result = False
status = EXECUTION_STATE_FAILED
output_message = f"Failed to connect to the {PROVIDER_NAME} server! Error is {e}"
References
- Ping action must use exact output messages for success and failure. (link)
content/response_integrations/google/cisco_orbital/core/CiscoOrbitalManager.py (27-37)
This function and others in the file are missing type annotations and are not using the Google Style Docstrings as required by the repository's style guide.
As per the style guide:
- (Line 80) All function parameters and return types must be annotated.
- (Line 88) Docstrings must follow the Google Style.
- (Line 91) Types should not be repeated in the
ArgsorReturnssections of the docstring.
For example, the __init__ method should be:
def __init__(
self,
api_root: str,
client_id: str,
client_secret: str,
verify_ssl: bool = True,
siemplify_logger=None
) -> None:
"""Initializes the CiscoOrbitalManager.
Args:
api_root: API root of the Cisco Orbital instance.
client_id: Client ID of the Cisco Orbital account.
client_secret: Client Secret of the Cisco Orbital account.
verify_ssl: If enabled, verify the SSL certificate for the connection.
siemplify_logger: Siemplify logger.
"""Please apply similar changes to the other functions in this file.
References
content/response_integrations/google/cisco_orbital/core/CiscoOrbitalParser.py (16)
Using a wildcard import (from .datamodels import *) is generally discouraged as it can lead to namespace pollution and makes it difficult to trace where classes and functions originate. It's better to explicitly import the required names.
from .datamodels import EndpointResult, TableData
content/response_integrations/google/cisco_orbital/core/CiscoOrbitalParser.py (20)
The functions in this class are missing type annotations for parameters and return values. According to the style guide (line 80), all function signatures must include type hints. For example:
def get_auth_token(self, raw_json: dict) -> str | None:Please add type hints to all functions in this file.
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/cisco_orbital/core/UtilsManager.py (23-28)
This function and others in the file are missing type annotations and are not using the Google Style Docstrings as required by the repository's style guide.
As per the style guide:
- (Line 80) All function parameters and return types must be annotated.
- (Line 88) Docstrings must follow the Google Style.
- (Line 91) Types should not be repeated in the
ArgsorReturnssections of the docstring.
For example:
import requests
from .exceptions import BadRequestException, CiscoOrbitalException
def validate_response(response: requests.Response, error_msg: str = "An error occurred") -> bool:
"""Validate response.
Args:
response: The response to validate.
error_msg: Default message to display on error.
Raises:
BadRequestException: If the response status code is 400.
CiscoOrbitalException: For other HTTP errors.
"""Please apply these standards throughout the file.
References
content/response_integrations/google/cisco_orbital/core/datamodels.py (29-39)
The __init__ method is missing type annotations for its parameters. According to the style guide (line 80), all function parameters must be annotated. The same applies to the __init__ method of the TableData class.
def __init__(
self,
raw_data: dict,
hostname: str | None,
local_ipv4: list[str],
local_ipv6: list[str],
external_ipv4: str | None,
error: str,
tables_data: list['TableData'],
limit: int | None,
):
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_active_response/actions/Ping.py (36)
The success message for the Ping action does not match the required format from the style guide (line 183): "Successfully connected to the {integration name} server with the provided connection parameters!".
siemplify.end("Successfully connected to the McAfeeActiveResponse server with the provided connection parameters.", True)
References
- Ping action must use exact output messages for success and failure. (link)
content/response_integrations/google/cisco_orbital/pyproject.toml (19)
The requires-python version should be specified as ">=3.11,<3.12" to ensure compatibility and prevent usage with untested future Python versions, as per the repository style guide (line 240).
requires-python = ">=3.11,<3.12"
References
- The
requires-pythonversion should be specified as>=3.11,<3.12. (link)
content/response_integrations/google/cyber_ark_credential_provider/actions/ping.py (33)
The failure message for the Ping action does not fully match the required format from the style guide (line 185). It should include the word "server". Please use the INTEGRATION_NAME constant to make the message dynamic and adhere to the standard: "Failed to connect to the {product name} server! Error is {error}".
self.error_output_message = f"Failed to connect to the {constants.INTEGRATION_NAME} server!"
References
- Ping action must use exact output messages for success and failure. (link)
content/response_integrations/google/cyber_ark_credential_provider/actions/ping.py (62-65)
The success message for the Ping action does not fully match the required format from the style guide (line 183). It should be: "Successfully connected to the {integration name} server with the provided connection parameters!".
self.output_message = (
f"Successfully connected to the {constants.INTEGRATION_NAME} server "
"with the provided connection parameters!"
)
References
- Ping action must use exact output messages for success and failure. (link)
content/response_integrations/google/cyber_ark_credential_provider/pyproject.toml (19)
The requires-python version should be specified as ">=3.11,<3.12" to ensure compatibility and prevent usage with untested future Python versions, as per the repository style guide (line 240).
requires-python = ">=3.11,<3.12"
References
- The
requires-pythonversion should be specified as>=3.11,<3.12. (link)
content/response_integrations/google/google_forms/actions/Ping.py (29)
The failure message for the Ping action does not match the required format from the style guide (line 185), which is "Failed to connect to the {product name} server! Error is {error}". The base Action class will append Reason: {e}, which differs from the required Error is {error} format. Consider overriding the exception handling or adjusting the message to produce the exact format.
References
- Ping action must use exact output messages for success and failure. (link)
content/response_integrations/google/cisco_orbital/actions/Ping.py (25)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/google_forms/pyproject.toml (19)
The requires-python version should be specified as ">=3.11,<3.12" to ensure compatibility and prevent usage with untested future Python versions, as per the repository style guide (line 240).
requires-python = ">=3.11,<3.12"
References
- The
requires-pythonversion should be specified as>=3.11,<3.12. (link)
content/response_integrations/google/cisco_orbital/core/datamodels.py (40)
The use of super(EndpointResult, self) is a legacy Python 2 pattern. In Python 3, you can and should use the simpler super() call.
super().__init__(raw_data)
content/response_integrations/google/cisco_orbital/actions/ExecuteQuery.py (36-59)
This function and others in the file (query_operation_status, main, check_entities_status, is_address_in_result) are missing type annotations and are not using the Google Style Docstrings as required by the repository's style guide.
As per the style guide:
- (Line 80) All function parameters and return types must be annotated.
- (Line 88) Docstrings must follow the Google Style.
- (Line 91) Types should not be repeated in the
ArgsorReturnssections of the docstring.
Here is an example of how this function could be documented:
from soar_sdk.SiemplifyAction import SiemplifyAction
from ..core.CiscoOrbitalManager import CiscoOrbitalManager
def start_operation(
siemplify: SiemplifyAction,
manager: CiscoOrbitalManager,
entities: list,
query: str,
name: str | None,
context: str | None,
limit: int | None,
hide_case_wall_table: bool | None,
expiration_unix: int,
) -> tuple[str, bool | str, str]:
"""Submit the query on endpoints.
Args:
siemplify: SiemplifyAction object.
manager: CiscoOrbitalManager object.
entities: The list of entities.
query: The query that needs to be executed.
name: The name for the query job.
context: The additional custom context fields that should be added to the job.
limit: Maximum number of results rows.
hide_case_wall_table: Whether to hide the case wall table.
expiration_unix: Unix epoch time the query will expire.
Returns:
A tuple containing the output message, result value, and status.
"""Please apply similar changes to the other functions in this file.
References
content/response_integrations/google/mc_afee_active_response/actions/Search.py (74)
There is a typo in the variable name output_massage. It should be output_message. This typo is repeated on lines 77 and 79.
output_message = f"Found {len(item_results)} results for search."
content/response_integrations/google/mc_afee_active_response/core/McAfeeActiveResponseManager.py (20)
This comment indicates Python 2.7, but the integration is configured for Python 3.11. Please update or remove this misleading comment.
content/response_integrations/google/cisco_orbital/actions/ExecuteQuery.yaml (50)
The result_example_path has a naming convention mismatch. According to the style guide (line 158), for an action file action_name.py, the example file should be named resources/action_name_JsonResult_example.json. For ExecuteQuery.py, this would be resources/ExecuteQuery_JsonResult_example.json. The current path is resources/execute_query_JsonResult_example.json.
result_example_path: resources/ExecuteQuery_JsonResult_example.jsonReferences
- The JSON example file must exist in the integration's
resources/directory and follow the naming conventionaction_name_JsonResult_example.json. (link)
content/response_integrations/google/mc_afee_active_response/pyproject.toml (19)
The requires-python version should be specified as ">=3.11,<3.12" to ensure compatibility and prevent usage with untested future Python versions, as per the repository style guide (line 240).
requires-python = ">=3.11,<3.12"
References
- The
requires-pythonversion should be specified as>=3.11,<3.12. (link)
content/response_integrations/google/mc_afee_atd/actions/CheckHash.py (29)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/actions/GetAnalyzerProfiles.py (26)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/actions/GetReport.py (36)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/actions/GetReport.py (49)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def fetch_scan_report_async() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/actions/Ping.py (25)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/actions/SubmitFile.py (30)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/actions/SubmitURL.py (40)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def main() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/actions/SubmitURL.py (145)
The function signature is missing a return type annotation. According to the style guide (line 80), all function return types must be annotated. This function should have a -> None return type.
def fetch_scan_report_async() -> None:
References
- All function parameters and return types must be annotated. (link)
content/response_integrations/google/mc_afee_atd/core/McAfeeATDManager.py (20)
This comment indicates Python 2.7, but the integration is configured for Python 3.11. Please update or remove this misleading comment.
content/response_integrations/google/mc_afee_atd/pyproject.toml (19)
The requires-python version should be specified as ">=3.11,<3.12" to ensure compatibility and prevent usage with untested future Python versions, as per the repository style guide (line 240).
requires-python = ">=3.11,<3.12"
References
- The
requires-pythonversion should be specified as>=3.11,<3.12. (link)
|
❌ Marketplace Validation Failed Click to view the full reportValidation Report🧩 IntegrationsPre-Build Stagemc_afee_active_response
site24x7
splash
websense
mc_afee_atd
cisco_orbital
|
|
❌ Marketplace Validation Failed Click to view the full reportValidation Report🧩 IntegrationsPre-Build Stagesite24x7
splash
mc_afee_active_response
mc_afee_atd
websense
cisco_orbital
|
migrate the integrations: