[#24] Client: Implement pool part1 #31

PavelGrossSpb · 2024-10-21T07:50:07Z

PavelGrossSpb commented

2024-10-21 07:50:07 +00:00

First iteration - base classes and methods

Signed-off-by: Pavel Gross p.gross@yadro.com

First iteration - base classes and methods Signed-off-by: Pavel Gross <p.gross@yadro.com>

PavelGrossSpb added 1 commit 2024-10-21 07:50:07 +00:00

[#24 ] Client: Implement pool part1

DCO / DCO (pull_request) Successful in 46s

Details

c9a75ea025

first iteration - base classes and methods

Signed-off-by: Pavel Gross <p.gross@yadro.com>

PavelGrossSpb requested review from dstepanov-yadro 2024-10-21 07:50:33 +00:00

PavelGrossSpb requested review from orikik 2024-10-21 07:50:33 +00:00

dstepanov-yadro requested changes 2024-10-23 07:15:38 +00:00

Dismissed

src/FrostFS.SDK.ClientV2/FrostFSClient.cs

					
				@ -483,0 +536,4 @@

				            return null;

				        }

				        catch (FrostFsException ex)

dstepanov-yadro commented

2024-10-23 07:00:34 +00:00

To tell the truth, it doesn't look pretty. Without examining the source code, it is unclear what the returned string means.

PavelGrossSpb commented

2024-11-01 07:43:58 +00:00

refactored

dstepanov-yadro marked this conversation as resolved

src/FrostFS.SDK.ClientV2/Poll/ClientWrapper.cs

					
				@ -0,0 +95,4 @@

				            await ScheduleGracefulClose().ConfigureAwait(false);

				        }

				#pragma warning disable CA2000 // Dispose objects before losing scope: will be disposed manually

dstepanov-yadro commented

2024-10-23 07:06:37 +00:00

Could be resolved with

var client FrostFSClient
try {
client = new(WrapperPrm);
...
Client = client;
client = null;
}
finally {
client?.Dispose();
}

Could be resolved with ``` var client FrostFSClient try { client = new(WrapperPrm); ... Client = client; client = null; } finally { client?.Dispose(); } ```

PavelGrossSpb commented

2024-11-01 07:45:29 +00:00

and why do I need a reference to disposed object?

dstepanov-yadro commented

2024-11-02 11:13:43 +00:00

I did not understand the question: where is the reference to disposed object?
There are 4 main steps:

Create temp reference to disposable object
Inside try block create an disposable object to this reference
Inside try block transfer ownership of disposable object from temp reference to new owner and set null to temp reference: now Client field holds reference to disposable object, not temp reference
Dispose disposable object if it is not null

I did not understand the question: where is the reference to disposed object? There are 4 main steps: 1. Create temp reference to disposable object 2. Inside `try` block create an disposable object to this reference 3. Inside `try` block transfer ownership of disposable object from temp reference to new owner and set `null` to temp reference: now `Client` field holds reference to disposable object, not temp reference 4. Dispose disposable object if it is not `null`

PavelGrossSpb commented

2024-11-05 07:39:04 +00:00

Got it. But it looks like a strange trick to avoid dispose pattern warning that follows another analizer warning about "always null variable". The suppressing comment is a good reminder about dispose necessity and keeps the code clean and clear.

dstepanov-yadro commented

2024-11-05 07:50:37 +00:00

I don't think it is a trick, but if you ok with it, I'm ok too.

src/FrostFS.SDK.ClientV2/Poll/Pool.cs

					
				@ -0,0 +296,4 @@

				        );

				    }

				    private FrostFSClient? Сonnection()

dstepanov-yadro commented

2024-10-23 07:12:39 +00:00

See warning

PavelGrossSpb commented

2024-11-01 07:45:47 +00:00

fixed

dstepanov-yadro marked this conversation as resolved

PavelGrossSpb added 1 commit 2024-11-01 07:30:52 +00:00

[#24 ] Client: Implement pool part2

DCO / DCO (pull_request) Successful in 46s

Details

ee20798379

Signed-off-by: Pavel Gross <p.gross@yadro.com>

PavelGrossSpb added 1 commit 2024-11-01 07:41:34 +00:00

[#24 ] Client: Implement pool part2

DCO / DCO (pull_request) Successful in 52s

Details

bff8d67867

Unicode fix

Signed-off-by: Pavel Gross <p.gross@yadro.com>

dstepanov-yadro approved these changes 2024-11-05 07:50:48 +00:00

PavelGrossSpb merged commit bff8d67867 into master

2024-11-05 11:44:51 +00:00

PavelGrossSpb deleted branch poll

2024-11-05 11:44:52 +00:00

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-sdk-csharp#31