Skip to content

Fixups to AzureFileShareService auth token - #819

Open
Brian Kroth (bpkroth) wants to merge 33 commits into
microsoft:mainfrom
bpkroth:fixes-to-pr-779
Open

Fixups to AzureFileShareService auth token #819
Brian Kroth (bpkroth) wants to merge 33 commits into
microsoft:mainfrom
bpkroth:fixes-to-pr-779

Conversation

@bpkroth

@bpkroth Brian Kroth (bpkroth) commented Jul 24, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Makes config errors load time apparent instead of runtime apparent.

Closes #818

Sergiy Matusevych (motus) and others added 24 commits July 8, 2024 16:05
@motus
use token-based credential instead of a storage key in Azure file sha…
2a6cd0d 
…re service
@motus
initialize auth service before azure fileshare in unit tests
951fdba
@motus
late initialization for azure fileshare _share_client; proper monkey …
9aeaa92 
…patching of the class for unit tests
@motus
remove storageAccountKey parameter from azure_fileshare fixture
54c308b
@motus
Merge branch 'main' into sergiym/svc/fs_token_auth
9b65bd7
@motus
Merge branch 'main' into sergiym/svc/fs_token_auth
101b593
@motus
Merge branch 'main' into sergiym/svc/fs_token_auth
71fae3c
@motus
Merge branch 'main' of https://github.com/microsoft/MLOS into sergiym…
4bc4c25 
…/svc/fs_token_auth
@motus
Merge branch 'sergiym/svc/fs_token_auth' of https://github.com/motus/…
289af50 
…MLOS into sergiym/svc/fs_token_auth
@motus
black formatting updates
ead96a1
@motus
Merge branch 'main' into sergiym/svc/fs_token_auth
a46bcd4
@motus
Merge branch 'main' into sergiym/svc/fs_token_auth
68c6d0d
@motus
Merge branch 'main' of https://github.com/microsoft/MLOS into sergiym…
99172ed 
…/svc/fs_token_auth
@motus
docformatter fixes
8078d70
@motus
Merge branch 'sergiym/svc/fs_token_auth' of https://github.com/motus/…
527ae66 
…MLOS into sergiym/svc/fs_token_auth
@motus
Merge branch 'main' into sergiym/svc/fs_token_auth
56704f6
@bpkroth
Merge branch 'main' into sergiym/svc/fs_token_auth
5af8b6f
@motus
Merge branch 'main' into sergiym/svc/fs_token_auth
378f073
@motus
formatting fixes
f2b47c6
@motus
remove git merge artifact
e9e48f2
@motus
more git merge fixes
2925ebb
@bpkroth
move sanity check into __init__
00ff3bf
@bpkroth
Merge remote-tracking branch 'origin/sergiym/svc/fs_token_auth' into …
fd45a8c 
…fixes-to-pr-779

Plus other fixes
@bpkroth
use the mock auth service
7895fe5
@bpkroth
Brian Kroth (bpkroth) requested a review from a team as a code owner July 24, 2024 20:27
@motus

Sergiy Matusevych (motus) commented Aug 3, 2024

Copy link
Copy Markdown
Member

I assume this update went to #820

@bpkroth

Brian Kroth (bpkroth) commented Aug 8, 2024

Copy link
Copy Markdown
Contributor Author

I assume this update went to #820

Parts of it did, but I don't think all of it. Especially the refresh logic.

Eu Jing Chua (@eujing) I see that you added some of the test fixes in. Have you tested #820 to confirm that it already handles credential refresh as necessary?

@bpkroth Brian Kroth (bpkroth) mentioned this pull request Aug 8, 2024
Merged
@bpkroth
Brian Kroth (bpkroth) enabled auto-merge (squash) August 8, 2024 22:04
@eujing

Eu Jing Chua (eujing) commented Sep 4, 2024

Copy link
Copy Markdown
Contributor

I assume this update went to #820

Parts of it did, but I don't think all of it. Especially the refresh logic.

Eu Jing Chua (@eujing) I see that you added some of the test fixes in. Have you tested #820 to confirm that it already handles credential refresh as necessary?

Yes I have been using the #820 changes, along with #837 for long running experiments. So at least for DefaultAzureCredential with the MI setup, the token refresh has been handled as expected.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@motus Sergiy Matusevych (motus) Awaiting requested review from motus

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

Fix issues in AzureFileShareService introduced by #779

3 participants

@bpkroth @motus @eujing