Conversation
Yavor16
force-pushed
the
adopt-azure-sdk
branch
2 times, most recently
from
3d7bc44 to
8abfeec
Compare
Yavor16
force-pushed
the
adopt-azure-sdk
branch
from
8abfeec to
0f289a1
Compare
|
| private int removeBlobsByFilter(Predicate<? super BlobItem> filter) { | ||
| Set<String> blobNames = getEntryNames(filter); | ||
| List<Boolean> deletedBlobsResults = new ArrayList<>(); | ||
|
|
||
| if (blobNames.isEmpty()) { | ||
| return 0; | ||
| } | ||
| for (String blobName : blobNames) { | ||
| BlobClient blobClient = containerClient.getBlobClient(blobName); | ||
| deletedBlobsResults.add(blobClient.deleteIfExists()); | ||
| } | ||
|
|
||
| deletedBlobsResults.removeIf(Boolean.FALSE::equals); | ||
|
|
||
| return deletedBlobsResults.size(); | ||
| } |
There was a problem hiding this comment.
Can we do it with a simple int counter and since deleteIfExists() returns true or false based on if the Blob was deleted, we would just increment the counter if true was returned and then we will return the counter at the end of the method?
Instead of creating a list of boolean objects and removing those which are false and then returning its size.
| .build(); | ||
| } | ||
|
|
||
| private static ObjectStoreServiceInfo buildGcoObjectStoreServiceInfo() { |
There was a problem hiding this comment.
typo in buildGcoObjectStoreServiceInfo. Gco -> Gcp
| int deletedFiles = fileStorage.deleteFilesModifiedBefore(LocalDateTime.ofInstant(Instant.ofEpochMilli(currentMillis - oldFilesTtl), | ||
| ZoneId.systemDefault())); |
There was a problem hiding this comment.
Since ZoneId.systemDefault() relies on the OS timezone, this test may behave differently across operating systems. Could that cause inconsistencies?
| ObjectStoreServiceInfo objectStoreServiceInfo = objectStoreServiceInfoOptional.get(); | ||
| return tryToCreateObjectStore(objectStoreServiceInfo, exceptions); | ||
| } | ||
| return Optional.empty(); |
There was a problem hiding this comment.
Would it make sense to add a log when the Optional is empty?
| ListBlobsOptions listBlobsOptions = new ListBlobsOptions(); | ||
| BlobListDetails blobListDetails = new BlobListDetails(); | ||
| blobListDetails.setRetrieveMetadata(true); | ||
| listBlobsOptions.setDetails(blobListDetails); |
There was a problem hiding this comment.
The listBlobOptions creation could be simplified by inlining like this:
ListBlobsOptions options = new ListBlobsOptions().setDetails(blobListDetails);
| public String getContainerUriEndpoint(Map<String, Object> credentials) { | ||
| if (!credentials.containsKey(CONTAINER_URI)) { | ||
| return null; | ||
| } | ||
| try { | ||
| URL containerUri = new URL((String) credentials.get(CONTAINER_URI)); | ||
| return new URL(containerUri.getProtocol(), containerUri.getHost(), containerUri.getPort(), "").toString(); | ||
| } catch (MalformedURLException e) { | ||
| throw new IllegalStateException(Messages.CANNOT_PARSE_CONTAINER_URI_OF_OBJECT_STORE, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
It looks like this method is only referenced by a protected method within the same class. Should we consider reducing its visibility from public to protected?
| private ObjectStoreServiceInfo createServiceInfoForAzure(Map<String, Object> credentials) { | ||
| return ImmutableObjectStoreServiceInfo.builder() | ||
| .provider(Constants.AZUREBLOB) | ||
| .credentials(credentials) | ||
| .build(); | ||
| } | ||
|
|
||
| private ObjectStoreServiceInfo createServiceInfoForGcp(Map<String, Object> credentials) { | ||
| return ImmutableObjectStoreServiceInfo.builder() | ||
| .provider(Constants.GOOGLE_CLOUD_STORAGE) | ||
| .credentials(credentials) | ||
| .build(); |
There was a problem hiding this comment.
Just a note:
Since we’re adopting different SDKs across environments, we could extract the shared Immutable builder logic into a helper method, as only the provider and credentials are parsed, and future SDK implementations may follow the same pattern.
3 participants
No description provided.