XSUP-28298/XSOAR-Mirroring #29869

sapirshuker · 2023-09-27T05:48:41Z

Contributing to Cortex XSOAR Content

Make sure to register your contribution by filling the contribution registration form

The Pull Request will be reviewed only after the contribution registration form is filled.

Status

In Progress
Ready
In Hold - (Reason for hold)

Related Issues

fixes: https://jira-hq.paloaltonetworks.local/browse/XSUP-28298

Description

Fixed an issue where the fetch-incidents command from XSOAR 8 to XSOAR 6 created infinite duplicates.

Must have

Tests
Documentation

…nt into XSUP-28298/XSOAR-Mirroring

eyalpalo

look at my comment, but overall can be sent to the customer

eyalpalo · 2023-09-27T08:52:48Z

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

-    next_run = {'last_fetch': (latest_created_time + timedelta(milliseconds=1))  # type: ignore[operator]
-                .strftime(XSOAR_DATE_FORMAT)}  # type: ignore[union-attr,operator]
-
+    next_run = {'last_fetch': (latest_created_time)  # type: ignore[operator]


What will happen in a case when we support milliseconds (from 6 to 8)? We need to keep supporting this situation.

We are supporting in milliseconds, I remove the timedelta(milliseconds=1) because I add the dedup, and it will prevent from duplication so we do not need to add time to the last fetch time.

eyalpalo · 2023-09-27T08:53:56Z

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

+    Returns:
+        Boolean: True if both datetime objects have the same time up to seconds, False otherwise.
+    """
+    return (x.year == y.year) and (x.month == y.month) and (x.day == y.day)\


can be replaced with x.replace(microsecond=0) and the same for y and simply compare them.

We have milliseconds in XSOAR6, so this comparison will be wrong in XSOAR6. Therefore, I only considered seconds and not milliseconds. Please explain your comment. I think I don't understand your meaning

…nt into XSUP-28298/XSOAR-Mirroring

AradCarmi

Nice!
See my comments bellow.

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

…nt into XSUP-28298/XSOAR-Mirroring

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

eyalpalo · 2023-10-02T12:11:46Z

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

+    incidents, last_fetched_incidents = get_and_dedup_incidents(client,last_fetched_incidents,
+                                                                  query, max_results, last_fetch)


Let's keep the convention of new lines and assignments by name for better readability (same as in the call you deleted)

eyalpalo · 2023-10-02T12:19:11Z

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

+    """ get incidents and dedup the incidents response.
+
+    Cases:
+    1.  Empty incidents list (no new incidents received from API response).
+        Meaning: Usually means there are not any more incidents to fetch at the moment.
+        Handle: Return empty list of incidents and the unchanged list of 'last_fetched_incidents' for next run.
+
+    2.  The response include incidents from the previous fetch cycle.
+        Meaning: There are potentially more incidents with the same timestamp.
+        Handle: Get more incidents util the number of the incident equal to the requested max fetch_limit.
+        Add the list of fetched incidents IDs to current 'last_fetched_incidents' from last run,
+        return list of new incidents and updated list of 'last_fetched_incidents' for next run.
+
+    3.  Most recent incident has later timestamp then other incidents in the response.
+        Meaning: This is the normal case where incidents in the response have different timestamps.
+        Handle: Return list of new incidents and a list of 'new_ids' containing only IDs of
+                incidents with identical latest time for next run.
+
+    Args:
+        incidents (list[dict]): List of incidents from the current fetch response.
+        last_fetched_incidents (list[dict]): List of IDs of incidents from last fetch cycle.
+
+    Returns:
+        tuple[list[dict], list[str]: The list of dedup incidents and ID list of incidents of current fetch.
+    """


Suggested change

""" get incidents and dedup the incidents response.

Cases:

1. Empty incidents list (no new incidents received from API response).

Meaning: Usually means there are not any more incidents to fetch at the moment.

Handle: Return empty list of incidents and the unchanged list of 'last_fetched_incidents' for next run.

2. The response include incidents from the previous fetch cycle.

Meaning: There are potentially more incidents with the same timestamp.

Handle: Get more incidents util the number of the incident equal to the requested max fetch_limit.

Add the list of fetched incidents IDs to current 'last_fetched_incidents' from last run,

return list of new incidents and updated list of 'last_fetched_incidents' for next run.

3. Most recent incident has later timestamp then other incidents in the response.

Meaning: This is the normal case where incidents in the response have different timestamps.

Handle: Return list of new incidents and a list of 'new_ids' containing only IDs of

incidents with identical latest time for next run.

Args:

incidents (list[dict]): List of incidents from the current fetch response.

last_fetched_incidents (list[dict]): List of IDs of incidents from last fetch cycle.

Returns:

tuple[list[dict], list[str]: The list of dedup incidents and ID list of incidents of current fetch.

"""

""" get incidents and dedup the incidents response.

Cases:

1. Empty incidents list (no new incidents received from API response).

Return empty list of incidents and the unchanged list of 'last_fetched_incidents'.

2. The response includes incidents from the previous fetch cycle, with the same timestamp.

Fetch incidents until the number of incidents is equal to the requested max fetch_limit.

Add the list of fetched incident IDs to the current 'last_fetched_incidents' from last run,

return a list of new incidents, and updated list of 'last_fetched_incidents'.

3. Most recent incident has a later timestamp than other incidents in the response.

Return a list of new incidents and a list of 'new_ids' containing only the IDs of

incidents with identical latest times for next run.

Args:

incidents (list[dict]): List of incidents from the current fetch response.

last_fetched_incidents (list[dict]): List of IDs of incidents from last fetch cycle.

Returns:

tuple[list[dict], list[str]: The list of dedup incidents and ID list of incidents of current fetch.

"""

eyalpalo · 2023-10-02T12:20:02Z

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

+    Returns:
+        tuple[list[dict], list[str]: The list of dedup incidents and ID list of incidents of current fetch.
+    """
+    last_fatched_incident = (dateparser.parse(last_fetched_incidents[-1]["created"])


Suggested change

last_fatched_incident = (dateparser.parse(last_fetched_incidents[-1]["created"])

last_fatched_incident = (dateparser.parse(last_fetched_incidents[-1].get("created"))

And please change all to get

eyalpalo · 2023-10-02T12:20:56Z

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

+        field="created",
+        page=page,
+        )
+        # Case 1: Empty response len(incidents) == 0


Suggested change

# Case 1: Empty response len(incidents) == 0

# Case 1: Empty response.

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

eyalpalo · 2023-10-02T13:32:16Z

Packs/XSOARmirroring/ReleaseNotes/2_0_20.md

+##### XSOAR Mirroring
+
+- Updated the Docker image to: *demisto/python3:3.10.13.74666*.
+- Fixed an issue where the ***fetch-incidents*** command from XSOAR 8 to XSOAR 6 created infinite duplicates.


Suggested change

- Fixed an issue where the ***fetch-incidents*** command from XSOAR 8 to XSOAR 6 created infinite duplicates.

- Fixed an issue where the ***fetch-incidents*** command created duplicates when fetching from XSOAR 8 to XSOAR 6.

…nt into XSUP-28298/XSOAR-Mirroring

eyalpalo

Nice job!

eyalpalo · 2023-10-10T14:13:50Z

Packs/XSOARmirroring/Integrations/XSOARmirroring/XSOARmirroring.py

-            incident_creation_time = dateparser.parse(incident["created"])
-            if incident_dict not in last_fetched_incidents:
+            incident_id = incident.get("id")
+            incident_creation_time = dateparser.parse(incident.get("created", datetime.now()))


Is there a chance that the created field won't be present? Im not sure that setting the datetime now as the creation time is the correct approach

* fix * update docker image * fix flake8 errors * fix flake8 mypy errors * fixs * add tests * cr review * replace incidents_last_fetch_ids with last_fetched_incidents * fix CR review, update docker * fix tests * add tests, fix flake8 errors * fix flake8 errors * fix flake8 errors * fix Flake8 errors * CR remarks

sapirshuker and others added 7 commits September 27, 2023 08:47

fix

499e079

update docker image

48cb969

Merge branch 'master' into XSUP-28298/XSOAR-Mirroring

d4fac4d

fix flake8 errors

c6c1b2b

Merge branch 'master' into XSUP-28298/XSOAR-Mirroring

54c9571

fix flake8 mypy errors

d009a9e

Merge branch 'XSUP-28298/XSOAR-Mirroring' of github.com:demisto/conte…

335f6b3

…nt into XSUP-28298/XSOAR-Mirroring

sapirshuker requested a review from AradCarmi September 27, 2023 07:44

Merge branch 'master' into XSUP-28298/XSOAR-Mirroring

cb00e97

sapirshuker requested a review from eyalpalo September 27, 2023 08:18

eyalpalo requested changes Sep 27, 2023

View reviewed changes

sapirshuker and others added 5 commits September 27, 2023 13:57

Merge branch 'master' into XSUP-28298/XSOAR-Mirroring

d301851

fixs

7d6b9f4

Merge branch 'XSUP-28298/XSOAR-Mirroring' of github.com:demisto/conte…

904fabd

…nt into XSUP-28298/XSOAR-Mirroring

add tests

02446c0

Merge branch 'master' into XSUP-28298/XSOAR-Mirroring

7c1fb91

AradCarmi reviewed Oct 1, 2023

View reviewed changes

sapirshuker added 3 commits October 1, 2023 18:55

cr review

5cf670f

Merge branch 'XSUP-28298/XSOAR-Mirroring' of github.com:demisto/conte…

7e7bd65

…nt into XSUP-28298/XSOAR-Mirroring

replace incidents_last_fetch_ids with last_fetched_incidents

d37dcb9

eyalpalo requested changes Oct 2, 2023

View reviewed changes

sapirshuker and others added 8 commits October 4, 2023 18:08

fix CR review, update docker

5eb94bb

Merge branch 'master' into XSUP-28298/XSOAR-Mirroring

96a19f2

fix tests

cb376c8

Merge branch 'XSUP-28298/XSOAR-Mirroring' of github.com:demisto/conte…

28c80c0

…nt into XSUP-28298/XSOAR-Mirroring

add tests, fix flake8 errors

9dd86fc

fix flake8 errors

0bff942

fix flake8 errors

641bf73

fix Flake8 errors

664ed94

sapirshuker requested a review from eyalpalo October 10, 2023 11:19

sapirshuker marked this pull request as ready for review October 10, 2023 12:09

eyalpalo approved these changes Oct 10, 2023

View reviewed changes

sapirshuker and others added 2 commits October 11, 2023 11:27

CR remarks

c3620be

Merge branch 'master' into XSUP-28298/XSOAR-Mirroring

6c1a500

sapirshuker added the docs-approved label Oct 11, 2023

sapirshuker merged commit acabd97 into master Oct 11, 2023
17 of 18 checks passed

sapirshuker deleted the XSUP-28298/XSOAR-Mirroring branch October 11, 2023 11:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

XSUP-28298/XSOAR-Mirroring #29869

XSUP-28298/XSOAR-Mirroring #29869

sapirshuker commented Sep 27, 2023

eyalpalo left a comment

eyalpalo Sep 27, 2023

sapirshuker Sep 27, 2023

eyalpalo Sep 27, 2023

sapirshuker Sep 27, 2023

AradCarmi left a comment

eyalpalo Oct 2, 2023

eyalpalo Oct 2, 2023

eyalpalo Oct 2, 2023

eyalpalo Oct 2, 2023

eyalpalo Oct 2, 2023

eyalpalo left a comment

eyalpalo Oct 10, 2023

		incidents, last_fetched_incidents = get_and_dedup_incidents(client,last_fetched_incidents,
		query, max_results, last_fetch)

	last_fatched_incident = (dateparser.parse(last_fetched_incidents[-1]["created"])
	last_fatched_incident = (dateparser.parse(last_fetched_incidents[-1].get("created"))

	# Case 1: Empty response len(incidents) == 0
	# Case 1: Empty response.

	- Fixed an issue where the *fetch-incidents* command from XSOAR 8 to XSOAR 6 created infinite duplicates.
	- Fixed an issue where the *fetch-incidents* command created duplicates when fetching from XSOAR 8 to XSOAR 6.

XSUP-28298/XSOAR-Mirroring #29869

XSUP-28298/XSOAR-Mirroring #29869

Conversation

sapirshuker commented Sep 27, 2023

Contributing to Cortex XSOAR Content

Status

Related Issues

Description

Must have

eyalpalo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AradCarmi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eyalpalo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment