Check if file exists in object store before reusing#1801
Check if file exists in object store before reusing#1801IvanBorislavovDimitrov wants to merge 2 commits intomasterfrom
Conversation
LMCROSSITXSADEPLOY-3411
IvanBorislavovDimitrov
force-pushed
the
check-file-upload
branch
from
f6c1e70 to
19a1c3b
Compare
| @@ -0,0 +1,2 @@ | |||
| com.google.api.client.googleapis.services.AbstractGoogleClient.level=SEVERE | |||
There was a problem hiding this comment.
Why do we set this to SEVERE in test logging? Is it to reduce noise?
There was a problem hiding this comment.
because now the GCP object store tests spam a lot and the build is getting too big without necessary information
this logging is only for tests
| public List<FileEntry> getExistingFileEntries(List<FileEntry> fileEntries) { | ||
| List<CompletableFuture<FileEntry>> existenceChecks = fileEntries.stream() |
There was a problem hiding this comment.
In the Azure implemenation there is validation if the fileEntries is empty. Do we need the same validation here?
There was a problem hiding this comment.
added it
| List<CompletableFuture<FileEntry>> existenceChecks = fileEntries.stream() | ||
| .map(fileEntry -> CompletableFuture.supplyAsync( | ||
| () -> existsInBlobStore(fileEntry), | ||
| virtualThreadExecutor)) | ||
| .toList(); |
There was a problem hiding this comment.
This part if duplicated. Can it be extracted somewhere and reused here so there is less duplication?
There was a problem hiding this comment.
done
| List<BlobId> blobIds = fileEntries.stream() | ||
| .map(fileEntry -> BlobId.of(bucketName, fileEntry.getId())) | ||
| .toList(); | ||
| List<Blob> blobs = storage.get(blobIds); |
There was a problem hiding this comment.
Why is this get request not executed in a thread like the other sdk-s?
There was a problem hiding this comment.
because it support multi fetch by multiple ids, others do not
| Storage mockedStorage = mock(Storage.class); | ||
| GcpObjectStoreFileStorage gcpFileStorage = gcpFileStorageWithMockedStorage(mockedStorage); |
There was a problem hiding this comment.
There’s some duplicated mock setup - should we pull it into a helper method?
There was a problem hiding this comment.
refactored the class
|
3 participants
LMCROSSITXSADEPLOY-3411