Syakima/CIAC-15740/forcepoint-add-refresh-command#43628
Conversation
🤖 AI-Powered Code Review AvailableYou can leverage AI-powered code review to assist with this PR! Available Commands:
|
Coverage Report
|
||||||||||||||||||||||||||||||
|
🤖 Analysis started. Please wait for results... |
🤖 AI Review DisclaimerThis review was generated by an AI-powered tool and may contain inaccuracies. Please be advised, and we extend our sincere apologies for any inconvenience this may cause. |
marketplace-ai-reviewer
left a comment
There was a problem hiding this comment.
Thanks for your contribution to the Forcepoint SMC integration! I've reviewed the PR and found a few logic and configuration adjustments needed.
Specifically, please ensure demisto.args() isn't evaluated at module load time, verify task success statuses when polling finishes, and fix the CommandResults initialization to use readable_output instead of outputs_prefix. You'll also need to align the task_ids argument name between the YAML and Python script, store it as a comma-separated string to avoid serialization issues, and update the pack metadata with the required XSOAR fields.
Let me know if you have any questions about these changes!
Additionally, please address the following file-level notes:
Packs/ForcepointSecurityManagementCenter/pack_metadata.json: - Theurlfield must be set tohttps://www.paloaltonetworks.com/cortexfor XSOAR-supported packs.- The vendor name 'Forcepoint' must be included in the
keywordslist. - The mandatory
createdkey is missing from the pack metadata.
@kamalq97 please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.
| demisto.debug("Completed task initiation.") | ||
| demisto.debug(f"Initiated refresh tasks for engines {', '.join(engine_names)}.") | ||
| demisto.debug(f"Tasks IDs: {task_ids}") | ||
| args["task_ids"] = task_ids |
There was a problem hiding this comment.
Store task_ids as a comma-separated string to avoid serialization issues.
There was a problem hiding this comment.
@TheL0L I would also recommend doing this (also ensures the order is kept and not changed).
There was a problem hiding this comment.
@kamalq97 Can we guarantee that the ids will not have commas within them, and if they do - escape them?
| assert result.scheduled_command._command == "forcepoint-smc-engine-refresh" | ||
| assert result.scheduled_command._args is not None | ||
| assert "task_ids" in result.scheduled_command._args | ||
| assert result.scheduled_command._args["task_ids"] == ["engine1", "engine2"] |
There was a problem hiding this comment.
Update test to expect a comma-separated string.
| assert result.scheduled_command._command == "forcepoint-smc-engine-refresh" | ||
| assert result.scheduled_command._args is not None | ||
| assert "task_ids" in result.scheduled_command._args | ||
| assert result.scheduled_command._args["task_ids"] == ["engine1"] |
There was a problem hiding this comment.
Update test to expect a comma-separated string.
kamalq97
left a comment
There was a problem hiding this comment.
@TheL0L Great job! Ensure all comments from me and the @marketplace-ai-reviewer are addressed. Please also fix all validation errors.
Once done, we'll proceed to the next steps.
| demisto.debug("Completed task initiation.") | ||
| demisto.debug(f"Initiated refresh tasks for engines {', '.join(engine_names)}.") | ||
| demisto.debug(f"Tasks IDs: {task_ids}") | ||
| args["task_ids"] = task_ids |
There was a problem hiding this comment.
@TheL0L I would also recommend doing this (also ensures the order is kept and not changed).
Co-authored-by: Kamal Qarain <45042524+kamalq97@users.noreply.github.com>
|
Validate summary Verdict: PR can be force merged from validate perspective? ✅ |
julieschwartz18
left a comment
There was a problem hiding this comment.
@TheL0L Doc review complete. Please check the comments and also regenerate the integration README file. In the meantime, adding the label docs-approved
Co-authored-by: julieschwartz18 <91824591+julieschwartz18@users.noreply.github.com>
|
This PR is marked as 'Stale' because it has been open for 30 days with no activity, it will be automatically closed in 15 days if no activity will be done. To reset the counter just remove the 'Stale' label or make changes to update this PR. If you wish this PR will never be marked as 'Stale' add the 'Ignore Stale' |
Related Issues
fixes: CIAC-15740
Description
Add polling command for refreshing engines.