Upgrade Azure Blobstore from legacy azure-sdk (11.0) to latest (12.27.1) #1316
Conversation
groldan
force-pushed
the
azureblob_upgrade_12.x
branch
from
2e8631f to
8e4059d
Compare
groldan
changed the title
groldan
force-pushed
the
azureblob_upgrade_12.x
branch
from
8e4059d to
92808d5
Compare
|
Please not the "Azure BlobStore Integration / Azure online" is failing cause my free account expired. I'm waiting for camptocamp to provision an account we can use. Meanwhile, feel free to run |
groldan
force-pushed
the
azureblob_upgrade_12.x
branch
from
92808d5 to
4de5a3d
Compare
groldan
changed the title
|
@aaime I understand that you might not be in a position to review this PR. Is there someone else that you could ask, so that we can get it into GeoServer 2.26.0 by Wednesday (2 days time)? |
|
I don't think there is anyone else with familiarity with this module. I'll try to review today. |
…7.0) Add azure storage blob 12.27.1 dependencies and upgrade the azure blobstore. Run Azurite-based integration tests against the latest Docker image version, remove legacy. Migrate to `com.azure:blobstorage:12.x`. The upgrade tries to do a 1:1 migration to the new azure-sdk API (in its blocking falvor) as much as possible, so that the integration tests serve to avoid regressions. A couple remarks: * There's no way to close the HTTPClient so AzureClient does not implement Closeable anymore. * Fix a bug in DeleteManager that always reported 0 objects deleted Upgrade `com.microsoft.azure:azure-storage-blob` to `com.azure:azure-storage-blob`.
groldan
force-pushed
the
azureblob_upgrade_12.x
branch
from
4de5a3d to
ad540d8
Compare
Thank you, could you let me know if it is approved, please @aaime so that I can continue with GeoTools 32.0 release. |
|
I did not manage to review yesterday, too many PRs and I had to do two releases for imageio-ext and jai-ext. Too much ongoing stuff today too... hopefully I'll get to it tomorrow |
| } catch (StorageException e) { | ||
| throw new RuntimeException("Failed to retrieve properties mappings", e); | ||
| } | ||
| // going big, retrieve all items, since at the end everything must be held in memory anyways |
There was a problem hiding this comment.
Looks like the new API does not have a limit to the number of blobs that are going to b retrieved, possibly because it's using streams. So the comment there does not make sense anymore, or does it?
| @@ -137,6 +137,7 @@ public void setProxyPort(Integer proxyPort) { | |||
| this.proxyPort = proxyPort; | |||
| } | |||
|
|
|||
| /** unused */ | |||
| public String getProxyUsername() { | |||
There was a problem hiding this comment.
Actually used by AzureClient.getProxyOptions
| @@ -145,6 +146,7 @@ public void setProxyUsername(String proxyUsername) { | |||
| this.proxyUsername = proxyUsername; | |||
| } | |||
|
|
|||
| /** unused */ | |||
| public String getProxyPassword() { | |||
There was a problem hiding this comment.
Actually used by AzureClient.getProxyOptions
Upgrade Azure Blobstore from legacy azure-sdk (11.0) to latest (12.27.1)
Upgrade
com.microsoft.azure:azure-storage-blobtocom.azure:azure-storage-blob.Add azure storage blob 12.27.1 dependencies and upgrade the azure blobstore.
Run Azurite-based integration tests against the latest Docker image version, remove legacy.
Migrate to
com.azure:blobstorage:12.x. The upgrade tries to do a 1:1 migration to the new azure-sdk API (in its blocking falvour) as much as possible, so that the integration tests serve to avoid regressions.A couple remarks:
implement Closeable anymore.