Add an 'exec' example. Clean up some websockets code. #114
Conversation
examples/exec/Exec.cs
Outdated
| { | ||
| internal class Exec | ||
| { | ||
| private static void Main(string[] args) |
There was a problem hiding this comment.
Note sure what version of C# this is built with but latest supports async main if that makes the code cleaner?
There was a problem hiding this comment.
Done (had to move to c# 7.1)
| @@ -59,10 +60,15 @@ public Kubernetes(KubernetesClientConfiguration config, params DelegatingHandler | |||
| } | |||
|
|
|||
| #if NET452 | |||
| ((WebRequestHandler) HttpClientHandler).ServerCertificateValidationCallback = | |||
| CertificateValidationCallBack; | |||
| ((WebRequestHandler) HttpClientHandler).ServerCertificateValidationCallback = (sender, certificate, chain, sslPolicyErrors) => | |||
| webSocketBuilder.ExpectServerCertificate(this.CaCert); | ||
| else |
There was a problem hiding this comment.
Aren't these mutually exclusive? I suppose throwing an exception would have been clearer when I first wrote this...
There was a problem hiding this comment.
Maybe? I sort of lean on "the customer is always right" or "the caller is always right"
If they ship us a config with a ca Cert and noVerifyTLS who are we to judge? We should obey their wishes.
There was a problem hiding this comment.
Yeah fair enough, that reasoning makes sense to me :)
My personal approach to API design is "forgiving, but also willing to point out when the caller may be making a mistake". If someone specifies both CaCert and SkipTlsVerify, which one did they really intend to specify? Is it intuitive to them which one we'd select when there's a conflict like this, and do we document this assumption anywhere?
But in this case, correct behaviour is dictated by the K8s client config spec which doesn't prevent them specifying both (so I agree with you that there's not much point in stopping them from doing so).
|
|
||
| return false; | ||
| } | ||
| return Kubernetes.CertificateValidationCallBack(sender, serverCertificate, certificate, chain, sslPolicyErrors); |
There was a problem hiding this comment.
Oops! Sorry, must have missed that :)
|
(sorry, review stuff doesn't show up well on mobile 😁) |
@tintoy
Thanks!