Pause and Resume #549

ognyan-kostadinov · 2023-03-13T09:51:44Z

No description provided.

bert-e · 2023-03-13T09:51:47Z

Hello ognyan-kostadinov,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2023-03-13T09:51:52Z

Incorrect fix version

The Fix Version/s in issue ZKUI-313 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

1.6.11

Please check the Fix Version/s of ZKUI-313, or the target
branch of this pull request.

codecov-commenter · 2023-03-16T12:04:43Z

Codecov Report

Merging #549 (8857f7e) into development/1.6 (b06bca2) will increase coverage by 0.86%.
The diff coverage is 56.07%.

@@                 Coverage Diff                 @@
##           development/1.6     #549      +/-   ##
===================================================
+ Coverage            51.94%   52.80%   +0.86%     
===================================================
  Files                  193      196       +3     
  Lines                 9239     9307      +68     
  Branches              2559     2574      +15     
===================================================
+ Hits                  4799     4915     +116     
+ Misses                4419     4371      -48     
  Partials                21       21

Impacted Files	Coverage Δ
src/js/ZenkoClient.ts	`40.47% <0.00%> (-6.75%)`	⬇️
src/react/backend/location/utils.tsx	`63.01% <ø> (+10.26%)`	⬆️
src/react/ui-elements/Help.tsx	`62.50% <ø> (ø)`
src/react/workflow/CreateWorkflow.tsx	`46.21% <ø> (ø)`
src/react/workflow/ReplicationForm.tsx	`64.37% <ø> (ø)`
src/react/backend/location/PauseAndResume.tsx	`54.28% <54.28%> (ø)`
src/react/actions/zenko.ts	`52.71% <60.00%> (ø)`
src/react/backend/location/queries.ts	`66.66% <66.66%> (ø)`
src/react/workflow/WorkflowState.tsx	`87.50% <87.50%> (ø)`
src/react/backend/location/Locations.tsx	`81.53% <100.00%> (+0.37%)`	⬆️

... and 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

JBWatenbergScality · 2023-03-17T16:42:38Z

src/react/actions/zenko.ts

I'm not sure any modification is necessary in this file.

JBWatenbergScality · 2023-03-17T16:45:14Z

src/react/backend/location/Locations.tsx

+  const {
+    data: instanceStatus,
+    status,
+    isFetching,
+  } = useQuery({
+    queryKey: ['instanceStatus', instanceId],
+    queryFn: () => {
+      return managementClient.getLatestInstanceStatus(
+        instanceId,
+      ) as InstanceStatus;
+    },
+    onError: (error) => {
+      dispatch(handleClientError(error));
+    },
+    refetchOnMount: false,
+    refetchOnWindowFocus: false,
+  });
+


As this instanceStatus query is only used by PauseAndResume component, I would suggest to move it there, similarly as for the invalidation logic

JBWatenbergScality · 2023-03-17T16:53:11Z

src/react/backend/location/PauseAndResume.tsx

+                dispatch(networkStart('Pausing Replication workflow'));
+                pauseReplicationSiteMutation.mutate(locationName);


IMO, this dispatch network start shall be handled by the mutation

JBWatenbergScality · 2023-03-17T16:53:27Z

src/react/backend/location/PauseAndResume.tsx

+                dispatch(networkStart('Pausing Async Metadata updates'));
+                pauseIngestionSiteMutation.mutate(locationName);


IMO, this dispatch network start shall be handled by the mutation

JBWatenbergScality · 2023-03-17T16:55:25Z

src/react/backend/location/Locations.tsx

+  const invalidateInstanceStatusQueryCache = async () => {
+    await queryClient.invalidateQueries(['instanceStatus', instanceId]);
+    await queryClient.refetchQueries(['instanceStatus', instanceId]);
+    dispatch(networkEnd());


This should also be called for the pause and resume mutation in case of failures

JBWatenbergScality · 2023-03-20T12:53:06Z

src/react/backend/location/Locations.tsx

+                processed only once the Workflow processes are resumed.Name of
+                the bucket/container created in the specific location (e.g.
+                RING, Azure, AWS S3, GCP...), and where buckets attached to that
+                location will store data.


Suggested change

processed only once the Workflow processes are resumed.Name of

the bucket/container created in the specific location (e.g.

RING, Azure, AWS S3, GCP...), and where buckets attached to that

location will store data.

processed only once the Workflow processes are resumed.

JBWatenbergScality · 2023-03-20T12:58:58Z

src/react/backend/location/PauseAndResume.tsx

+    await queryClient.invalidateQueries(['instanceStatus', instanceId]);
+    await queryClient.refetchQueries(['instanceStatus', instanceId]);


Why do we need both ? I assume await queryClient.refetchQueries(['instanceStatus', instanceId]); should be enough here.

In addition once the query would be extracted we could retrieve the query id from the extracted query directly

JBWatenbergScality · 2023-03-20T13:00:02Z

src/react/backend/location/PauseAndResume.tsx

+  } = useQuery({
+    queryKey: ['instanceStatus', instanceId],
+    queryFn: () => {
+      return managementClient?.getLatestInstanceStatus(
+        instanceId,
+      ) as InstanceStatus;
+    },
+    onError: (error) => {
+      dispatch(handleClientError(error));
+    },
+    refetchOnMount: false,
+    refetchOnWindowFocus: false,
+  });


I would extract this query into a queries object similarly as we do for other features. That will enable simpler reuse of the queries later on

JBWatenbergScality · 2023-03-20T13:06:56Z

src/react/backend/location/__tests__/PauseAndResume.test.tsx

+    await waitForElementToBeRemoved(() => screen.queryByTestId('loader'), {
+      timeout: 8000,
+    });


Can we use below option instead?

Suggested change

await waitForElementToBeRemoved(() => screen.queryByTestId('loader'), {

timeout: 8000,

});

await waitForElementToBeRemoved(() => screen.queryByText('Loading'), {

timeout: 8000,

});

ognyan-kostadinov · 2023-03-21T11:27:43Z

/approve

bert-e · 2023-03-21T11:27:50Z

Incorrect fix version

The Fix Version/s in issue ZKUI-313 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

1.6.11

Please check the Fix Version/s of ZKUI-313, or the target
branch of this pull request.

The following options are set: approve

ChengYanJin · 2023-03-22T13:06:50Z

/status

bert-e · 2023-03-22T13:06:54Z

Status

Status report is not available.

The following options are set: approve

bert-e · 2023-03-22T13:07:04Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/1.6

The following branches will NOT be impacted:

development/1.4
development/1.5

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2023-03-22T13:15:00Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/1.6

The following branches have NOT changed:

development/1.4
development/1.5

Please check the status of the associated issue ZKUI-313.

Goodbye ognyan-kostadinov.

WIP

5b13407

Refactoring and introducing tests

60da25a

Added column tooltip and paused indication component in Replication form

94ac3e0

ognyan-kostadinov marked this pull request as ready for review March 17, 2023 13:43

JBWatenbergScality reviewed Mar 17, 2023

View reviewed changes

Addressed PR comments

9260f7e

JBWatenbergScality reviewed Mar 20, 2023

View reviewed changes

Addressed more PR comments

8857f7e

JBWatenbergScality approved these changes Mar 21, 2023

View reviewed changes

bert-e merged commit 5f7b49c into development/1.6 Mar 22, 2023

bert-e deleted the improvement/ZKUI-313/pause-and-resume-replication branch March 22, 2023 13:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pause and Resume #549

Pause and Resume #549

ognyan-kostadinov commented Mar 13, 2023

bert-e commented Mar 13, 2023

bert-e commented Mar 13, 2023

codecov-commenter commented Mar 16, 2023 •

edited

Loading

JBWatenbergScality Mar 17, 2023

JBWatenbergScality Mar 17, 2023

JBWatenbergScality Mar 17, 2023

JBWatenbergScality Mar 17, 2023

JBWatenbergScality Mar 17, 2023

JBWatenbergScality Mar 20, 2023

JBWatenbergScality Mar 20, 2023

JBWatenbergScality Mar 20, 2023

JBWatenbergScality Mar 20, 2023

JBWatenbergScality Mar 20, 2023

ognyan-kostadinov commented Mar 21, 2023

bert-e commented Mar 21, 2023

ChengYanJin commented Mar 22, 2023

bert-e commented Mar 22, 2023

bert-e commented Mar 22, 2023

bert-e commented Mar 22, 2023

		dispatch(networkStart('Pausing Replication workflow'));
		pauseReplicationSiteMutation.mutate(locationName);

		dispatch(networkStart('Pausing Async Metadata updates'));
		pauseIngestionSiteMutation.mutate(locationName);

		await queryClient.invalidateQueries(['instanceStatus', instanceId]);
		await queryClient.refetchQueries(['instanceStatus', instanceId]);

Pause and Resume #549

Pause and Resume #549

Conversation

ognyan-kostadinov commented Mar 13, 2023

bert-e commented Mar 13, 2023

Hello ognyan-kostadinov,

bert-e commented Mar 13, 2023

Incorrect fix version

codecov-commenter commented Mar 16, 2023 • edited Loading

Codecov Report

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

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

ognyan-kostadinov commented Mar 21, 2023

bert-e commented Mar 21, 2023

Incorrect fix version

ChengYanJin commented Mar 22, 2023

bert-e commented Mar 22, 2023

Status

bert-e commented Mar 22, 2023

In the queue

bert-e commented Mar 22, 2023

codecov-commenter commented Mar 16, 2023 •

edited

Loading