-
Notifications
You must be signed in to change notification settings - Fork 812
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
Basic Teamcity agent - only supports guest authentication, default branch #1171
Conversation
Thanks @AirbornePorcine for your contribution, we'll review that shortly and provide some ideas for the test suite ;) |
def __init__(self, name, init_config, agentConfig): | ||
AgentCheck.__init__(self, name, init_config, agentConfig) | ||
self.last_build_ids = {} | ||
if self.init_config.get("server", None) is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The init_config
is used for parameters at the check level, the server
variable is an instance level parameter thus it should be setup in each instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also .get()
returns None by default, so you can remove the None
default return value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Leo. Thanks for all the feedback on this! I'll get things fixed up.
I specifically made server an init_config parameter as I figured most people would use this check on a single CI server (TeamCity isn't typically clustered or anything), but for multiple build configs on said server. If that doesn't make sense, I can move it to an instance parameter for sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! If nobody could possibly use 2 server instances, then it probably makes sense.
* Reduced timeouts to 30s * Checking status code before parsing JSON from TeamCity * Utilizing snake_case for Pythonic config file parameters * Removing 'teamcity' tag
…ppear unavailable on the windows agent.
self._initialize_if_required(instance_name, build_configuration) | ||
|
||
request = requests.get( | ||
"http://{}/guestAuth/app/rest/builds/?locator=buildType:{},sinceBuild:id:{},status:SUCCESS".format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This string formatting syntax is not compatible with python 2.6
Python 2.6.9 (unknown, Sep 9 2014, 15:05:12)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> a = "{}".format("hello")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: zero length field name in format
Could you replace it with this syntax please ?
Python 2.6.9 (unknown, Sep 9 2014, 15:05:12)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.39)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> a = "{0}".format("hello")
Thanks again @AirbornePorcine your code has been merged with the #1308 PR. |
Basic support for the TeamCity CI server (https://www.jetbrains.com/teamcity/) in a similar style to the Jenkins agent check.
Unfortunately I'm not very well-versed in Python testing techniques so I wasn't able to write any tests here. Help with that would certainly be appreciated.