microsoft / hi-ml

HI-ML toolbox for deep learning for medical imaging and Azure integration
https://aka.ms/hi-ml
MIT License
269 stars 60 forks source link

BUG: Fix for duplicate authentication #878

Closed ant0nsc closed 1 year ago

ant0nsc commented 1 year ago

Resolve issues with AML SDK v1/v2: Even in the v2 code path we are creating v1 workspaces, leading to duplicate authentication requests. To achieve that, a couple of other changes were necessary:

Also added more detailed logging for dataset creation, and a commandline flag to control logging level.

codecov[bot] commented 1 year ago

Codecov Report

Merging #878 (46ac0db) into main (683def9) will increase coverage by 0.32%. The diff coverage is 44.44%.

Impacted file tree graph

Flag Coverage Δ
hi-ml 83.28% <83.33%> (+0.26%) :arrow_up:
hi-ml-azure 26.99% <30.18%> (+1.60%) :arrow_up:
hi-ml-cpath 75.91% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
hi-ml-azure/src/health_azure/himl_download.py 0.00% <ø> (ø)
hi-ml-azure/src/health_azure/logging.py 0.00% <ø> (ø)
hi-ml-azure/src/health_azure/datasets.py 27.39% <22.22%> (-8.63%) :arrow_down:
hi-ml-azure/src/health_azure/utils.py 29.39% <25.64%> (+0.58%) :arrow_up:
hi-ml/src/health_ml/runner.py 86.20% <57.14%> (-0.68%) :arrow_down:
hi-ml-azure/src/health_azure/himl.py 41.32% <80.00%> (+14.30%) :arrow_up:
...th/src/health_cpath/scripts/mount_azure_dataset.py 70.37% <100.00%> (ø)
hi-ml/src/health_ml/experiment_config.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

fepegar commented 1 year ago

Can you please summarise the changes in this PR so it's a bit easier to review?

fepegar commented 1 year ago

I will take a look at this tomorrow.