Skip to content

[fs] Add support for azure https blob urls#12917

Merged
danking merged 13 commits intohail-is:mainfrom
daniel-goldstein:azure-https-2
Apr 24, 2023
Merged

[fs] Add support for azure https blob urls#12917
danking merged 13 commits intohail-is:mainfrom
daniel-goldstein:azure-https-2

Conversation

@daniel-goldstein
Copy link
Contributor

@daniel-goldstein daniel-goldstein commented Apr 21, 2023

CHANGELOG: ABS blob URIs in the format of https://<ACCOUNT_NAME>.blob.core.windows.net/<CONTAINER_NAME>/<PATH> are now supported. The hail-az scheme for referencing blobs in ABS is now deprecated and will be removed in an upcoming release.

This PR introduces the https addressing of blobs in ABS and phases out hail-az. The test suite converts completely to testing https, but both schemes are still supported. We can have confidence that this did not break completely break the hail-az scheme because our test bucket configuration is still using hail-az (and must until this PR is merged. So some of the test suite + all the service backend tests are flexing the https code path, and then the inter_cloud tests are flexing the hail-az code path. After this merges, we'll need the following PRs

  • Update the azure terraform to use https instead of hail-az and apply the changes
  • Remove support for the hail-az scheme. This will be a breaking change as the copy tool and batch worker will stop being able to transfer files and logs for that scheme. It seems like there is large support for dropping this scheme entirely though so I'd rather make this change while there is little use on azure.

@daniel-goldstein daniel-goldstein changed the title Azure https 2 [fs] Add support for azure https blob urls Apr 21, 2023
Copy link
Contributor

@danking danking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@danking danking merged commit a1a4028 into hail-is:main Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants