alan-turing-institute / data-safe-haven

https://data-safe-haven.readthedocs.io
BSD 3-Clause "New" or "Revised" License
61 stars 15 forks source link

Fixes for AzureSdk blob handling #2132

Closed jemrobinson closed 3 months ago

jemrobinson commented 3 months ago

:white_check_mark: Checklist

:vertical_traffic_light: Depends on

n/a

:arrow_heading_up: Summary

:closed_umbrella: Related issues

n/a

:microscope: Tests

Tested on an existing SRE

github-actions[bot] commented 3 months ago

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  data_safe_haven/external/api
  azure_sdk.py 121-127, 129-130, 156, 176-189, 191-192, 961, 972, 1308
Project Total  

This report was generated by python-coverage-comment-action

jemrobinson commented 3 months ago

I think that would be fine. If you're happy with these changes otherwise, maybe the easiest solution is to merge this as-is, merge release-v5.0.0 into your branch for #2129 and then make the blob name optional there. What do you think?

jemrobinson commented 3 months ago

Or actually, separate this function into a blob_service_client and blob_client function. I can do that here if you'd prefer?

jemrobinson commented 3 months ago

@craddm : Does f14ae787e fix your issue?

craddm commented 3 months ago

@craddm : Does https://github.com/alan-turing-institute/data-safe-haven/commit/f14ae787e75ed81aa4218ef857899b2c18f4d67c fix your issue?

Tested this with some edits to #2129 and it works!

jemrobinson commented 3 months ago

Great - are we OK to merge this one then?

craddm commented 3 months ago

LGTM