A brand new API - pyteamcity.future
#37
Conversation
Most of them were in integration tests which I moved to separate files that are not checked in.
Add missing `import requests` and use `UnauthorizedError` from our `exceptions` module
| TEAMCITY_HOST | ||
| TEAMCITY_USER | ||
| TEAMCITY_PASSWORD | ||
| commands = py.test {posargs:-v --cov=pyteamcity --cov-report=term-missing test_pyteamcity.py pyteamcity/future/tests/unit/} |
There was a problem hiding this comment.
I suggest only having py.test here and moving the rest in pytest.ini (and move the path under testpaths, not addopts). This way one can just type py.test without running tox and it will behave the same.
There was a problem hiding this comment.
Oh cool! Glad to have you looking at this, @aconrad!
There was a problem hiding this comment.
Thank you! See e0de037
as suggested by @aconrad at #37 (comment)
to make it clearer that it's legacy and probably won't be further developed.
Allows getting server info like web_url, version, etc.
| def server_info(self): | ||
| url = self.base_url + '/app/rest/server' | ||
| res = self.session.get(url) | ||
| res.raise_for_status() |
There was a problem hiding this comment.
Wouldn't you want to wrap this into your own exception instead of exposing the exception of the underlying http library (requests)?
There was a problem hiding this comment.
Yeah. Only problem is there are so many possible HTTP errors and I hate to have to create my own exception for each one.
There was a problem hiding this comment.
You already have HTTPError, don't you? You can use this as your base exception for all http exceptions. Later, if you find yourself needing to expose more specific http errors then you can create new exceptions that inherit HTTPError -- this won't be backwards incompatible for existing user code.
There was a problem hiding this comment.
Good point. Went ahead and did this in d9967bb. Thanks!
* .travis.yml: Post coverage to couverture.io * .travis.yml: Skip couverture for flake8 builds
so this is not blank file, which might be confusing couverture.io?
|
@msabramo I think I fixed the issue we were running into, please let me know if you see anything else not behaving properly with regards to coverage. |
|
Thanks, @aconrad! |
| env | sort | ||
| if [ "$TOXENV" == "flake8" ]; then | ||
| return | ||
| fi |
There was a problem hiding this comment.
We could also test for the presence of a coverage.xml file, that would scale better.
There was a problem hiding this comment.
Yeah that's even better. Maybe we should put this in a separate shell script like couverture.sh? Maybe couverture could offer a link to download one.
There was a problem hiding this comment.
Yes, I'm thinking about doing this eventually.
so hopefully folks don't waste time on using it or adding features to it.
Simpler, more efficient, and works with paths that reference artifacts inside zip file artifacts.
and default port to 443 for https
Goal here is to create a brand new API in
pyteamcity.futurethat is much more flexible and to have nicer code that is easier to work with. The old code encourages adding a zillion methods for different ways of filtering. The new code has an API with a smaller number of methods that are more consistent and more flexible in terms of filtering. It is modeled loosely after the Django ORM API.The plan is to make this available as
pyteamcity.futurefor a few releases while still retaining the old legacy API aspyteamcity, so existing code using that API still works. Then after a few releases, I would remove the legacy API and renamepyteamcity.futuretopyteamcityand it would be the only supported API.