[MetricsAdvisor] Overwriting patch serialization methods to support sending null (3) #22464
Conversation
kinelski
added
the
Client
ghost
added
the
| if (Optional.IsDefined(CrossMetricsOperator)) | ||
| { | ||
| writer.WritePropertyName("crossMetricsOperator"); | ||
| writer.WriteStringValue(CrossMetricsOperator.Value.ToString()); | ||
| } |
There was a problem hiding this comment.
Not sending null on this one because the service does not support this scenario. They only accept "send value" / "not send" for this property.
|
|
||
| namespace Azure.AI.MetricsAdvisor | ||
| { | ||
| internal static class Utf8JsonWriterExtensions |
There was a problem hiding this comment.
This class was already present in #22463. No changes.
|
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, please perform following steps: For data-plane/track 2 SDKs Issue the following command as a pull request comment:
For track 1 management-plane SDKsPlease open a separate PR and to your service SDK path in this file. Once that PR has been merged, you can re-run the pipeline to trigger the verification. |
.../Azure.AI.MetricsAdvisor/src/Models/CodeGenInternal/PatchModels/WebhookHookParameterPatch.cs
Show resolved
Hide resolved
|
|
||
| var hookToUpdate = disposableHook.Hook as WebNotificationHook; | ||
|
|
||
| hookToUpdate.Username = "username"; |
There was a problem hiding this comment.
maybe I just got confused with all the changes, but could you remind me what is the purpose here?
my understanding is that password-type things are not returned by the service, right? so here are u validating .. what?
There was a problem hiding this comment.
Refer to my comment in the other question. Only secrets in data sources are not returned.
There was a problem hiding this comment.
so should this be set to null? or why is ti that u assign a value and then the returned from the service is empty?
There was a problem hiding this comment.
It should be set to null. Fixed it.
There was a problem hiding this comment.
Ran it locally and noticed we have trouble validating the certification key, since the service does not seem to accept mock values (already tracked by #17485). Also, I was not taking sanitization of passwords into account for tests. Fixed it.
Part of #21509.
This PR is a continuation of #22457 and #22463 (to be reviewed first).
We're adding the custom patch serialization to Alert Configurations and Notification Hooks.