Fix files and events replicators

[maudeburkhalter]

• Add a 7th argument and set it to None for copy_events and copy_files functions in order to make them work again
• Background: the addition of a 7th arg to replication.thread for use in TimeSeries has broken other resource copy_* fn-s.

[cognite-cicd]

Checkmarx scan completed with the following findings

Lines	Severity	Category	File	Link

[gaetan-h]

Looks good to me!

[sceniclife]

Hey, thanks for the hotfix.

The bug also exists for Files also since it use the same replicator.thread function. (7th argument).
While adding a 7th argument and setting to null prevents the issue, it's not exactly neat. Now we have a dst_ts variable designated for time-series floating into events.py and possibly files.py.

The dst_ts feature was added to timeseries as List of specific time series external ids to replicate. To fix this issue correctly, we should add the feature to events.py and files.py respectively.

1. Change the variable dst_ts to src_filter in timeseries.py and replication.py
2. Change the parameters for replication.threads to

def thread(
    num_threads: int,
    copy,
    src_objects: List[Union[Event, FileMetadata, TimeSeries]],
    src_id_dst_obj: Dict[int, Union[Event, FileMetadata, TimeSeries]],
    src_dst_ids_assets: Dict[int, int],
    project_src: str,
    replicated_runtime: int,
    client: CogniteClient,
    src_filter: Optional[List[Union[Event, FileMetadata, TimeSeries]]] = None,
):

3. Add the logic and variable src_filter onto events.py and files.py to filter by external_id (similar to time_series.py)

if target_external_ids:
    ts_src = client_src.time_series.retrieve_multiple(external_ids=target_external_ids, ignore_unknown_ids=True)
    try:
        ts_dst = client_dst.time_series.retrieve_multiple(external_ids=target_external_ids, ignore_unknown_ids=True)
    except CogniteNotFoundError:
        ts_dst = TimeSeriesList([])
else:
    ts_src = client_src.time_series.list(limit=None)
    ts_dst = client_dst.time_series.list(limit=None)
    logging.info(f"There are {len(ts_src)} existing time series in source ({project_src}).")
    logging.info(f"There are {len(ts_dst)} existing time series in destination ({project_dst}).")

Then it should all work with the intended features.

[gaetan-h]

Thanks, it was fixed for both files and events. Thanks for the suggestions. Feel free to open a PR if you want, otherwise I will try to have someone look at it when possible, to have a better solution

…

On Wed, 11 Nov 2020, 17:38 QuocViet Le, ***@***.***> wrote: Hey, thanks for the hotfix. The bug also exists for Files also since it use the same replicator.thread function. (7th argument). While adding a 7th argument and setting to null prevents the issue, it's not exactly neat. Now we have a dst_ts variable designated for time-series floating into events.py and possibly files.py. The dst_ts feature was added to timeseries as *List of specific time series external ids to replicate*. To fix this issue correctly, we should add the feature to events.py and files.py respectively. 1. Change the variable dst_ts to src_filter in timeseries.py and replication.py 2. Change the parameters for replication.threads to def thread( num_threads: int, copy, src_objects: List[Union[Event, FileMetadata, TimeSeries]], src_id_dst_obj: Dict[int, Union[Event, FileMetadata, TimeSeries]], src_dst_ids_assets: Dict[int, int], project_src: str, replicated_runtime: int, client: CogniteClient, src_filter: Optional[List[Union[Event, FileMetadata, TimeSeries]]] = None, ): 3. Add the logic and variable src_filter onto events.py and files.py to filter by external_id (similar to time_series.py) if target_external_ids: ts_src = client_src.time_series.retrieve_multiple(external_ids=target_external_ids, ignore_unknown_ids=True) try: ts_dst = client_dst.time_series.retrieve_multiple(external_ids=target_external_ids, ignore_unknown_ids=True) except CogniteNotFoundError: ts_dst = TimeSeriesList([]) else: ts_src = client_src.time_series.list(limit=None) ts_dst = client_dst.time_series.list(limit=None) logging.info(f"There are {len(ts_src)} existing time series in source ({project_src}).") logging.info(f"There are {len(ts_dst)} existing time series in destination ({project_dst}).") Then it should all work with the intended features. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#134 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOHWHMWP5NCOK5CNVBJJJT3SPK4XZANCNFSM4TRU7ULQ> .

[maudeburkhalter]

@sceniclife yes, this was just a quick fix to restore the broken behavior. I agree that a more elegant solution should be implemented; I came across the bug while needing to use the replicator and pushed the fix to have a working version out there.