Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[identity] Unpin msal-common and msal-browser dependencies #15338

Merged
4 commits merged into from
May 20, 2021

Conversation

witemple-msft
Copy link
Member

@witemple-msft witemple-msft commented May 19, 2021

Dependency analysis was showing that we are pulling in multiple copies of msal-common in the install footprint of identity. We were minor-pinned to msal-common 4.0.x (latest is 4.3.0) and hard pinned to a specific version of msal-browser. However, msal-browser itself carries a dependency on msal-common, and so our install footprint is currently pulling in three different versions of msal-common. Letting these dependencies roll forward within the same major version should help.

image

CC @HarshaNalluru @sadasant -- is this something that will give us recorder pain?

@witemple-msft witemple-msft requested a review from sadasant May 19, 2021 17:37
@witemple-msft witemple-msft requested a review from schaabs as a code owner May 19, 2021 17:37
@ghost ghost added the Azure.Identity label May 19, 2021
Copy link
Contributor

@sadasant sadasant left a comment

Choose a reason for hiding this comment

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

This is completely reasonable. Thank you for doing this! - though I'm really hoping nothing breaks since minor updates on MSAL have proven to cause havoc on our testing suite. Crossing my fingers!

@witemple-msft witemple-msft requested a review from bterlson as a code owner May 19, 2021 17:44
Copy link
Member

@HarshaNalluru HarshaNalluru left a comment

Choose a reason for hiding this comment

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

Our browser tests don't care about the msal-browser since we rely on the CSC. We need to figure out a way to test the IBC with our SDKs soon.
Coming to the node tests, as long as msal doesn't drastically change the auth mechanism/calls, nothing should break the unit tests. Even if we see any such disruptive changes, we'd try to have mock calls in the recorder itself as much as possible to make it less disruptive.

@HarshaNalluru
Copy link
Member

HarshaNalluru commented May 19, 2021

#15339 should fix the node 16 failure.
Once that PR is merged, please re-run the job.

@ramya-rao-a
Copy link
Contributor

Oh look at that... merge conflicts on the lock file... what a surprise...

@ghost
Copy link

ghost commented May 20, 2021

Hello @witemple-msft!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit f0973fa into Azure:master May 20, 2021
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-js that referenced this pull request Nov 9, 2022
Adding resourceconnector - Appliance GA API version (Azure#20681)

* Adding resourceconnector - appliance GA API verson

* Update swagger PR to add ArtifactURL and Another SSHKey type - ScopesAccessKey

* Commiting previous API Version 2022-04-15-preview

* Commiting latest API Version 2022-10-27

* Update to v3 types:

* Adding words to custom-words.txt

* run prettier check

* Use v3 for appliance resource type too

* resolve merge conflicts in custom words.txt

* add in readmes

* Add blockchain to latest profile

* Add additional types

* track2 modify eventhub readme.go.md (Azure#15338)

* track2 modify operationalinsights readme.go.md (Azure#15354)

* track2 modify security readme.go.md (Azure#15357)

* track2 modify eventgrid readme.go.md (Azure#15358)

* [Hub Generated] Public private branch 'dev-azure-kusto-Microsoft.Kusto-2021-08-27' (Azure#15905)

* Adds base for updating Microsoft.Kusto from version stable/2021-01-01 to version 2021-08-27

* Updates readme

* Updates API version in new specs and examples

* Swagger changes

* it1

* hh

* it2

* it3

* it5

* it6

* it7

* readme

* it10

* it11

* it11

* it 12

* it 13

* prettier

* python

* allowedIpRangeList - remove default

* bassel changes

* checkNameAvailability

* fix lint

* checkName

* fix

* managedPrivate check name

* prettier

* Update readme.python.md

* add attached enum

* Dor comments

* fix description

* readOnly

Co-authored-by: msyyc <[email protected]>

* update the format to int64 for creationtimestamp and expirationtimestamp

* Updating Delete example to include location header for 202 response

* fix header in delete example

Co-authored-by: Mark Cowlishaw <[email protected]>
Co-authored-by: JiahuiPeng <[email protected]>
Co-authored-by: hila levi <[email protected]>
Co-authored-by: msyyc <[email protected]>
Co-authored-by: Aditi Malladi <[email protected]>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants