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

[appconfig] Fix core-client mismatch with AppConfig #24366

Merged
merged 1 commit into from
Jan 4, 2023

Conversation

xirzec
Copy link
Member

@xirzec xirzec commented Jan 4, 2023

Packages impacted by this PR

@azure/app-configuration

Describe the problem that is addressed by this PR

CI was failing because appconfig was pulling in the public version of core-http-compat (1.3.0) which causes an interesting quirk in rush: core-http-compat pulled in the public version of core-client (1.6.1) while appconfig itself pulled in the repo version of core-client (1.7.0).

Since AppConfig was removing and re-adding the deserializationPolicy from the pipeline, this meant the deserializationPolicy from 1.7.0 was not able to access the OperationSpec stored by the ServiceClient from 1.6.1.

In practice this issue is less likely to occur in the wild since most package managers would consolidate down to a single version of core-client and there are not pre-release local versions to choose from.

The fix in this PR is to correctly leverage our internal pipeline options to avoid having to remove and re-add the deserializationPolicy from the pipeline which avoids the problem entirely.

@xirzec xirzec added Client This issue points to a problem in the data-plane of the library. App Configuration Azure.ApplicationModel.Configuration labels Jan 4, 2023
@xirzec xirzec requested a review from HarshaNalluru as a code owner January 4, 2023 21:47
@xirzec xirzec self-assigned this Jan 4, 2023
@azure-sdk
Copy link
Collaborator

API change check

API changes are not detected in this pull request.

Copy link
Member

@jeremymeng jeremymeng left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@xirzec xirzec merged commit 5ced438 into Azure:main Jan 4, 2023
@xirzec xirzec deleted the fixAppConfigDeserialization branch January 4, 2023 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App Configuration Azure.ApplicationModel.Configuration Client This issue points to a problem in the data-plane of the library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants