[teamcity] Add ssl_validation flag to check #2091
Conversation
|
Thanks @jslatts We'll review more in details for our 5.7.0 release. |
| @@ -21,7 +21,7 @@ def __init__(self, name, init_config, agentConfig, instances=None): | |||
| # Keep track of last build IDs per instance | |||
| self.last_build_ids = {} | |||
|
|
|||
| def _initialize_if_required(self, instance_name, server, build_conf): | |||
| def _initialize_if_required(self, instance_name, server, build_conf, ssl_validation): | |||
There was a problem hiding this comment.
Looks like you forgot to pass the ssl_validation flag when calling _initialize_if_required
There was a problem hiding this comment.
@irabinovitch sorry, missed the original notification. I added it in. Do you want me to rebase the PR against current master? I collapsed the fix into the original commit.
jslatts
force-pushed
the
teamcity_ssl_verify
branch
from
8f815e1
to
ad9b6d3
Compare
| @@ -22,6 +22,9 @@ instances: | |||
| # rather than just that a successful build happened | |||
| # is_deployment: true | |||
|
|
|||
| # Optional, this turns of ssl ceritificate validation. Defaults to True. | |||
There was a problem hiding this comment.
Small typos here: turns of ssl ceritificate validation -> turns off ssl certificate validation
|
Hi @jslatts! I've added two comments, could you address them? Also, could you only keep your commits in this PR? The other 3 commits seem to be unrelated. Apart from that your changes look good to me. Thanks! |
- Same as flag added for nginx in issue DataDog#1782
jslatts
force-pushed
the
teamcity_ssl_verify
branch
from
ad9b6d3
to
865c64a
Compare
|
Fixed the typo and added the _is_affirmative call. Also rebased against current master. No idea how the other commits ended up in there. It looks clean now. Let me know if there is anything else. |
|
Thanks again @jslatts ! Merging. |
[teamcity] Add ssl_validation flag to check
It would be helpful to have the ability to disable SSL validation in the teamcity check, same idea as issue #1782.
I went ahead and added a flag in with the same name as the nginx check. I did not try and write an integration test. Is a test needed for this?