[App Config] Migrate to core-v2 #20766
Conversation
ghost
added
|
API changes have been detected in |
|
This pull request is protected by Check Enforcer. What is Check Enforcer?Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass. Why am I getting this message?You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged. What should I do now?If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows: What if I am onboarding a new service?Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment: |
… harshan/app-config/15809
… harshan/issue/20813
HarshaNalluru
changed the title
There was a problem hiding this comment.
@HarshaNalluru Can you tell where you are getting the CompatResponse from?
| ); | ||
| return webResource; | ||
| } | ||
| export function createAppConfigKeyCredentialPolicy( |
There was a problem hiding this comment.
nit: just appConfigKeyCredentialPolicy()
| result: T | ||
| ): asserts result is T & HttpResponseField<any> { | ||
| if (!hasUnderscoreResponse(result)) { | ||
| throw new Error( |
There was a problem hiding this comment.
I wonder if we should instead log this (and maybe have it fail our tests as well) -- basically I would be a little concerned about breaking someone's usage of app-config if they don't rely on _response. Maybe if it doesn't exist we can attach a message on _response that says to file a bug? :)
We can take this as a follow-up issue, though
There was a problem hiding this comment.
oh that's a great point.
sdk/appconfiguration/app-configuration/src/internal/synctokenpolicy.ts
Outdated
Show resolved
Hide resolved
|
|
||
| if (hasUnderscoreResponse(kvp)) { | ||
| Object.defineProperty(response, "_response", { | ||
| enumerable: false, |
There was a problem hiding this comment.
not technically required (all flags default to false), but I'm fine with making it explicit
sdk/appconfiguration/app-configuration/src/appConfigCredential.ts
Outdated
Show resolved
Hide resolved
| @@ -197,3 +200,323 @@ export async function assertThrowsAbortError( | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| export function toWebResourceLike(request: PipelineRequest): WebResourceLike { | |||
There was a problem hiding this comment.
do we need full copies of all this compat code as test helpers? what are we validating?
There was a problem hiding this comment.
I was lazy. Removed everything and simplified.
Tests are happy, I'm happy.
… harshan/app-config/15809
Co-authored-by: Jeff Fisher <[email protected]>
…olicy.ts Co-authored-by: Jeff Fisher <[email protected]>
…alluru/azure-sdk-for-js into harshan/app-config/15809
Packages impacted by this PR
@azure/app-configurationIssues associated with this PR
Fixes #6484 with #20817
Fixes #15809
Describe the problem that is addressed by this PR
Migrating app-config to core-v2
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Generated code with
generate:clientscript to get new core-v2 support.Checklists