-
Notifications
You must be signed in to change notification settings - Fork 39
add versions, environments and improve test coverage #125
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
Conversation
…ilt-in requests package
@@ -309,12 +308,10 @@ def list_languages(self, client): | |||
return table | |||
|
|||
def getBuildLogs(self, user, algo, client): | |||
api_response = client.algo(user + '/' + algo).build_logs() | |||
|
|||
if "error" in api_response: |
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.
what will happen here if there is an error, now that you remove error handling?
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.
it will just raise the error as per the build_logs() endpoint, previously there was no error handling in the internal build_logs() endpoint, if it was a 500 we needed that to be handled here
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.
Overall looks good. Nice refactoring on the getJsonHelper.
Note that this is a breaking python api change so will need major version. I support the changes though.
This PR does a bunch (sorry!)
It strips out most of the openapi spec generated client logic; and instead hardcodes API paths.
It also replaces the autogenerated class objects with json response objects, but keeps error handling the same.
This unbreaks the following paths with gitlab / bitbucket based SCMs:
algo.versions()
algo.info()
algo.builds()
We also added the following endpoints:
algo.versions()
algo.exists()
more test coverage may be required.